ES|QL - fix dense vector enrich bug#139774
Conversation
carlosdelest
added
>bug
Team:Analytics
|
Hi @carlosdelest, I've created a changelog YAML for you. |
elasticsearchmachine
added
the
Team:Search Relevance
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
elasticsearchmachine
removed
the
Team:Analytics
alex-spies
left a comment
alex-spies
left a comment
There was a problem hiding this comment.
Not a full review, but can we also add dense_vector to the enrich-related tests in AllSupportedFieldsTestCase? This guards against regressions in FROM | ENRICH and ROW | ENRICH queries where the enrich fields are of all kinds of types. Currently, we exclude dense vector fields
…tor-enrich-bugfix # Conflicts: # x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/AllSupportedFieldsTestCase.java
alex-spies
left a comment
alex-spies
left a comment
There was a problem hiding this comment.
Thanks @carlosdelest !
I'm afraid you'll need to add test-release and run AllSupportedFieldsIT in release mode. I expect failures, esp. in testRowEnrich, because the tests run all kinds of bwc setups, and supportedInEnrich currently doesn't take the versions into account. This method controls the test expectations as well as the setup, and while it was previously generally true that we wouldn't expect dense vector fields in the enrich policy, now we do but only if the cluster already has the bugfix. To decide the right test expectation, we'll probably have to take the presence of the ENRICH_DENSE_VECTOR_BUGFIX capability into account in these tests.
alex-spies
left a comment
alex-spies
left a comment
There was a problem hiding this comment.
Apart from my comments on AllSupportedFieldsTestCase, the change looks right to me. Thanks, Carlos!
…tor-enrich-bugfix
|
@elasticmachine test this please |
@alex-spies I set the |
…tor-enrich-bugfix # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
…nrich-bugfix' into bugfix/esql-dense-vector-enrich-bugfix
@alex-spies after-Christmas bump 🎄 😉 |
…tor-enrich-bugfix
alex-spies
left a comment
alex-spies
left a comment
There was a problem hiding this comment.
Nice, thanks for adding dense_vector to the all supported fields tests, @carlosdelest !
|
@elasticsearchmachine run elasticsearch-ci/part-2 |
💚 Backport successful
|
…i-project-tests * upstream/main: (23 commits) Fix `testAckListenerReceivesNacksIfPublicationTimesOut` (elastic#140514) Reduce priority of clear-cache tasks (elastic#139685) Add docs and tests about `StreamOutput` to memory (elastic#140365) ES|QL - dense_vector support for COUNT, PRESENT, ABSENT aggregator functions (elastic#139914) Add release notes for v9.2.4 release (elastic#140487) Add release notes for v9.1.10 release (elastic#140488) Add conncectors release notes for 9.1.10, 9.2.4 (elastic#140499) Add parameter support in PromQL query durations (elastic#139873) Improve testing of STS credentials reloading (elastic#140114) Fix zstd native binary publishing script to support newer versions (elastic#140485) Add FlattenedFieldBinaryVsSortedSetDocValuesSyntheticSourceIT (elastic#140489) Store fallback match only text fields in binary doc values (elastic#140189) [DiskBBQ] Use the new merge executor for intra-merge parallelism (elastic#139942) ESQL: introduce support for mapping-unavailable fields (elastic#140463) Add ESNextOSQVectorsScorerTests (elastic#140436) Disable high cardinality tests on release builds (elastic#140503) ESQL: TRange timezone support (elastic#139911) Directly compressing `StreamOutput` (elastic#140502) ES|QL - fix dense vector enrich bug (elastic#139774) Use CrossProjectModeDecider in RemoteClusterService (elastic#140481) ...
3 participants
Closes #137699
ENRICH in ES|QL didn't work when using dense_vector as a column for adding as part of the ENRICH command.
This PR addresses it by:
EnrichResultBuilderForFloatthat takes care ofdense_vectorblocks, which are FLOAT internally