Skip to content

Conversation

kanoshiou
Copy link
Contributor

This PR fixes #119950 where an IN query includes NULL values with non-NULL DataType appearing within the query range. An expression is considered NULL when its DataType is NULL or it is a Literal with a value of null.

@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.1.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Mar 28, 2025
@gbanasiak gbanasiak added the :Analytics/ES|QL AKA ESQL label Mar 28, 2025
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed needs:triage Requires assignment of a team area label labels Mar 28, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@idegtiarenko idegtiarenko self-assigned this Apr 8, 2025
@kanoshiou
Copy link
Contributor Author

Thanks for review @idegtiarenko!

I believe you should manually ping buildkite for a CI test since this PR is submitted by an external contributor.

@idegtiarenko
Copy link
Contributor

buildkite test this

@kanoshiou
Copy link
Contributor Author

I forgot to add a new capability for the csv test, now it has been included in 2fd4e60.

@astefan astefan requested a review from bpintea April 10, 2025 13:45
Copy link
Contributor

@bpintea bpintea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, added minor notes only. 🙏

required_capability: filter_in_converted_null
FROM employees
| WHERE emp_no in (10021, 10022, null::int)
| KEEP emp_no, first_name, last_name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a trailing SORT on emp_no? The results might get rearranged otherwise and fail the test.

In in = new In(
EMPTY,
new FieldAttribute(Source.EMPTY, "field", new EsField("suffix", DataType.KEYWORD, Map.of(), true)),
Arrays.asList(ONE, new Literal(Source.EMPTY, null, DataType.INTEGER), THREE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we randomise the data type here?

@kanoshiou
Copy link
Contributor Author

Thank you for your review @bpintea! Tests have been updated in 9e4100a.

@bpintea
Copy link
Contributor

bpintea commented Apr 11, 2025

buildkite test this

@bpintea
Copy link
Contributor

bpintea commented Apr 11, 2025

Thanks, @kanoshiou!

@bpintea bpintea merged commit 4cc21b6 into elastic:main Apr 11, 2025
19 checks passed
nik9000 pushed a commit to nik9000/elasticsearch that referenced this pull request May 1, 2025
This PR fixes elastic#119950 where an `IN` query includes `NULL` values with non-NULL `DataType` appearing within the query range. An expression is considered `NULL` when its `DataType` is `NULL` or it is a `Literal` with a value of `null`.
@nik9000 nik9000 added the v8.19.0 label May 1, 2025
nik9000 added a commit that referenced this pull request May 1, 2025
* ESQL: Allow the data type of `null` in filters (#118324) * Allow the data type of `null` in filters * ESQL: Fix `NULL` handling in `IN` clause (#125832) This PR fixes #119950 where an `IN` query includes `NULL` values with non-NULL `DataType` appearing within the query range. An expression is considered `NULL` when its `DataType` is `NULL` or it is a `Literal` with a value of `null`. * Revert formatting --------- Co-authored-by: kanoshiou <73424326+kanoshiou@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >bug external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.1.0

6 participants