SQL: Fix QlIllegalArgumentException with non-foldable date range queries#142386
SQL: Fix QlIllegalArgumentException with non-foldable date range queries#142386luigidellaquila merged 8 commits intoelastic:mainfrom
Conversation
luigidellaquila
added
>bug
:Analytics/SQL
elasticsearchmachine
added
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
elasticsearchmachine
added
v9.3.1
v8.19.12
v9.2.6
and removed
branch:9.2
branch:8.19
branch:9.3
labels
|
Hi @luigidellaquila, I've created a changelog YAML for you. |
astefan
left a comment
astefan
left a comment
There was a problem hiding this comment.
Apart from my one comment related to the change, I would advise checking EQL code as well (since this code change is in common shared code between EQL and SQL) to see if/how this is affecting it. If necessary add tests for EQL as well. Thank you
| return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler)); | ||
| } | ||
| // Fall back to script query for non-foldable bounds (e.g., DATE_ADD with field reference) | ||
| return new ScriptQuery(r.source(), r.asScript()); |
There was a problem hiding this comment.
Here don't you need to also call wrapIfNested?
Ie
Query q = new ScriptQuery(r.source(), r.asScript());
return wrapIfNested(q, r.value());
There was a problem hiding this comment.
Thanks for checking @astefan
I would advise checking EQL code as well
EQL is more restrictive than SQL, the optimizer can create Range objects, but only starting from expressions like field >= expression, and the right hand side expression is always checked and required to be foldable (if it's not, you'll get a Comparisons against fields are not (currently) supported).
Here don't you need to also call wrapIfNested?
That's a good point, let me check and add it.
There was a problem hiding this comment.
I added wrapIfNested for correctness, but I think there is no practical way to make it happen in production. The verifier prevents functions from using nested fields, see https://github.com/elastic/elasticsearch/blob/main/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java#L848, so I couldn't reproduce the original problem with a nested field.
…on-sliced-reindex * upstream/main: (110 commits) Add search task watchdog to log hot threads on slow search (elastic#142746) Fix return_intermediate_results query param on get async search results (elastic#142875) Mute org.elasticsearch.compute.operator.exchange.BatchDriverTests testSinglePageSingleBatch elastic#142895 Cancel reindex body always has status (elastic#142766) Fix built-in roles sync losing updates (elastic#142433) ESQL: Clarify docs and add csv test for WHERE in STATS (elastic#133629) Fix and unmute ReindexResumeIT (elastic#142788) Fix broken release notes Mute org.elasticsearch.benchmark.vector.scorer.VectorScorerOSQBenchmarkTests testSingleScalarVsVectorized {p0=384 p1=4 p2=NIO p3=COSINE} elastic#142883 ES|QL: fix Generative tests for commands that don't change the output schema (elastic#142864) Mute org.elasticsearch.benchmark.vector.scorer.VectorScorerOSQBenchmarkTests testSingleScalarVsVectorized {p0=1024 p1=1 p2=NIO p3=DOT_PRODUCT} elastic#142881 SQL: Fix QlIllegalArgumentException with non-foldable date range queries (elastic#142386) Add more errors to the allowed_errors with github issue links (elastic#142862) ESQL: reapply "NDJSON datasource" (elastic#142855) Add implementation to update service settings method for Alibaba Cloud Search service (elastic#142738) Mute org.elasticsearch.snapshots.SnapshotShutdownIT testStartRemoveNodeButDoNotComplete elastic#142871 Mute org.elasticsearch.snapshots.SnapshotShutdownIT testDeleteSnapshotWithPausedShardSnapshots elastic#142870 Mute org.elasticsearch.snapshots.SnapshotShutdownIT testAbortSnapshotWhileRemovingNode elastic#142869 Mute org.elasticsearch.snapshots.SnapshotShutdownIT testRemoveNodeDuringSnapshot elastic#142868 ES|QL: Guard exponential_histogram TO_STRING against too large inputs (elastic#140718) ...
3 participants
Fixing a NullPointerException due to the attempt to fold a range expression that is not foldable.
We'll translate it into a Painless script instead
Developed using AI-assisted tooling
Fixes: #137365