Skip to content

ESQL: Fix NULL handling in IN clause #125832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Apr 11, 2025

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 <[email protected]>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ESQL: NPE when converting NULL in IN clause
6 participants