ESQL: Allow the data type of null in filters

[kanoshiou]

Allow filter null in ESQL.

Closes #116351

[kanoshiou]

I think I should write another test where the result is empty when where null is used, but I don't know which file to write it in.

[elasticsearchmachine]

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

[fang-xing-esql]

I think I should write another test where the result is empty when where null is used, but I don't know which file to write it in.

Thank you for contributing to Elasticsearch.

You may add more complicated queries in some of the csv-spec files under here to validate the query returns correct results. For example, the null.csv-spec could be a good place to add null related queries.

And you may also add more complicated queries to LogicalPlanOptimizerTests to validate the query plans sent from coordinator node to the data nodes are as expected.

As this change affects the behavior of the targeting queries on different Elasticsearch versions, backward compatibility should also be taken into account.

[kanoshiou]

Thank you for your detailed guidance @fang-xing-esql !

I have added some tests, but I am not sure if they meet the requirements. Please let me know if any changes are needed.

[fang-xing-esql]

Thank you for your detailed guidance @fang-xing-esql !

I have added some tests, but I am not sure if they meet the requirements. Please let me know if any changes are needed.

Thank you for adding more tests @kanoshiou! We have a couple of more suggestions and then this PR is in a good shape.

• NULL is a DataType in ES|QL, and ES|QL also has other DataTypes that can contain NULL values. A predicate can be boolean type or null type, the other DataTypes are not supported. It will be great to have some negative test cases to cover more DataTypes with NULL values. Here is one example, and you may add more to cover more DataTypes like numeric, string, datetime etc. FROM t | where null::string. These negative tests can be added in VerifierTests.

• Users can provided null directly under WHERE clause, and they can also provide null in EVAL. It will be great to have tests to cover scenarios like this - FROM t | EVAL x = null | WHERE x, and mixed with different DataTypes .

• There are a couple of places in Verifier that check the predicate types. One is under checkFilterConditionType that you have fixed, the other is under checkInvalidNamedExpressionUsage, it will be great to refactor them into one place so that it is easier to maintain the consistency going forward.

[kanoshiou]

Thank you for your guidance and support on the PR, @fang-xing-esql!

If my test cases have any omissions, please let me know. I'm happy to add them. 😊

[fang-xing-esql]

Thank you @kanoshiou! I left a small comment, the rest looks good.

[kanoshiou]

Thank you for reviewing, @fang-xing-esql !

I've updated the branch according to your comment.

[fang-xing-esql]

buildkite test this

[kanoshiou]

Hi @fang-xing-esql ,

I tried the command in buildkite to reproduce the failure on my local machine, but no exception was thrown. This is quite puzzling.

[fang-xing-esql]

buildkite test this

[fang-xing-esql]

buildkite test this

[kanoshiou]

I believe the failed tests are unrelated to my code changes, @fang-xing-esql .

I’ve merged the latest code into my branch. The first failed test case has been muted by #119510, and I have tested the remaining two failed tests; they passed successfully.

Hope the tests pass this time. 🤞

[fang-xing-esql]

buildkite test this

[fang-xing-esql]

buildkite test this

[fang-xing-esql]

Thank you for your contribution to elasticsearch @kanoshiou! Looks good to me.

[kanoshiou]

Thank you for approving this PR @fang-xing-esql! Just a gentle reminder in case you forget to click the merge button.