ESQL: Add javadoc that explains version-aware planning#139706
ESQL: Add javadoc that explains version-aware planning#139706elasticsearchmachine merged 6 commits intoelastic:mainfrom
Conversation
craigtaverner
left a comment
There was a problem hiding this comment.
Excellent addition. I had some comments and questions.
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Versioned.java
Outdated
Show resolved
Hide resolved
| * don't support. Backporting this to 9.4.9 is likely fine, as the minimum transport version of a mixed 9.4.x/9.5+ setup | ||
| * (cluster or CCS) will be 9.4.x, so this optimization will be (correctly) disabled on pre-9.4.9 nodes. | ||
| * <p> | ||
| * However, if we also backport to 9.3.7, we have a problem: A 9.3.7/9.4.x/9.5+ setup will have a minimum transport version |
There was a problem hiding this comment.
Do we have a solution, or we leave that to the author to figure out when the face this issue? Or is this a case where we simple cannot backport fixes to more than one minor, ever?
There was a problem hiding this comment.
To solve this we'd have to be aware of not only the global minimum version, but at least of the minimum version per cluster. (Assuming that each cluster isn't in a rolling upgrade from a version with support to a version without support for a given version-aware planning change. Theoretically, knowing all different versions that participate is needed to solve this in all situations.)
I didn't want to write this here because I don't think that's complexity we want to introduce. For the time being, I'd say that backporting a version-aware planning change more than 1 minor version is just not supported.
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Versioned.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java
Outdated
Show resolved
Hide resolved
| /** | ||
| * Combines all components necessary for the coordinating node to plan and execute an ESQL query. | ||
| * <p> | ||
| * Note that this is not a session in the traditional sense of a stateful connection. This will |
There was a problem hiding this comment.
Note that this is not a session, I wonder if it justifies to rename the class to something else.
There was a problem hiding this comment.
I think so. Thing is, we also have PlanExecutor that has overlap in responsibilities, because EsqlSession also has methods to execute ESQL queries. Maybe the execution stuff should wholly be moved to PlanExecutor and what remains in EsqlSession would then be query planning logic - justifying a rename to QueryPlanner, maybe?
There was a problem hiding this comment.
In any case, that's something for another PR IMHO.
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Outdated
Show resolved
Hide resolved
* upstream/main: (253 commits) Adds ST_SIMPLIFY geo spatial function (elastic#136309) Take control of max clause count verification in Lucene searcher (elastic#139752) [ML] Unmute Inference Test (elastic#139765) Parameterize the vector operation benchmark tests (elastic#139735) Fix node reduction pushdown tests for release tests (elastic#139548) Fix flakiness in TSDataGenerationHelper (elastic#139759) CPS: Copy existing resolved index expressions when constructing a new `SearchRequest` from an existing one (elastic#139596) Add release notes for v9.1.9 release (elastic#139674) Add lucene query for wildcards on high cardinality keyword fields. (elastic#139746) Suppress Tika entitlement warnings from AWT (elastic#139711) Check field storage when synthetic source is enabled, in tests (elastic#139715) Refactor VectorSimilarityType to know about its corresponding Function (elastic#139678) Merge fixes from patch branch back into main (elastic#139721) Define native bulk operations for vector square distance (elastic#139198) Use LongUpDownCounter for Linked Project Error Metrics (elastic#139657) ESQL: Add javadoc that explains version-aware planning (elastic#139706) Add helper to pick node for reindex relocation (elastic#139081) Fix auth serialization randomized version test (elastic#139182) ES|QL - Add parsing, preanalysis and analysis timing information to profile (elastic#139540) Mute org.elasticsearch.persistent.ClusterPersistentTasksCustomMetadataTests testMinVersionSerialization elastic#139741 ...
Part of #137534