Skip to content

Conversation

ioanatia
Copy link
Contributor

tracked by #123389

Handles the case where the score column or the group column (_fork) has multi values.
In this case, we cannot properly assign a score value, so we assign a score value of null.
We also issue a warning header when we encounter this case.

This is consistent with what other commands or functions are doing.
I based a lot of the multi values handling implementation on the ChangePointOperator.

There is still the case where the key columns have multi values.
I have not included that here, since this proves to be a bit more tricky to handle and it's directly tied to how STATS handles grouping by multi valued columns.

@ioanatia ioanatia added >non-issue Team:Search - Relevance The Search organization Search Relevance team v9.2.0 :Search Relevance/ES|QL Search functionality in ES|QL labels Sep 25, 2025
Comment on lines +335 to +336
int scorePosition = layout.get(fuse.score().id()).channel();
int discriminatorPosition = layout.get(fuse.discriminator().id()).channel();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually the canonical way to get the position of the block representing the values of an attribute 🙈 .

@ioanatia ioanatia marked this pull request as ready for review September 26, 2025 07:41
@elasticsearchmachine elasticsearchmachine added Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch and removed Team:Search - Relevance The Search organization Search Relevance team labels Sep 26, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

Copy link
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Fun with MultiValues and NULLs!

;

warning:Line 6:3: evaluation of [FUSE LINEAR SCORE BY my_score GROUP BY my_fork] failed, treating result as null. Only first 20 failures recorded.
warning:Line 6:3: java.lang.IllegalArgumentException: group column contains multivalued entries; assigning null scores
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get rid of the java.lang.IllegalArgumentException in the warning? I don't think it provides additional info to users

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I am not sure, I just followed what we already have - there are many examples of warnings that include IllegalArgumentException, some examples:

660:x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec:warningRegex:java.lang.IllegalArgumentException: milliSeconds \[-1457696696640\] are before the epoch in 1970 and cannot be converted to nanoseconds
662:x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec:warningRegex:java.lang.IllegalArgumentException: milliSeconds \[-1457696696640\] are before the epoch in 1970 and cannot be converted to nanoseconds
664:x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec:warningRegex:java.lang.IllegalArgumentException: milliSeconds \[-1457696696640\] are before the epoch in 1970 and cannot be converted to nanoseconds
666:x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec:warningRegex:java.lang.IllegalArgumentException: milliSeconds \[-1457696696640\] are before the epoch in 1970 and cannot be converted to nanoseconds
668:x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec:warningRegex:java.lang.IllegalArgumentException: milliSeconds \[-1457696696640\] are before the epoch in 1970 and cannot be converted to nanoseconds
670:x-pack/plugin/esql/qa/testFixtures/src/main/resources/union_types.csv-spec:warningRegex:java.lang.IllegalArgumentException: milliSeconds \[-1457696696640\] are before the epoch in 1970 and cannot be converted to nanoseconds
674:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
676:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
678:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
680:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
682:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
684:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
686:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
688:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
690:x-pack/plugin/esql/qa/testFixtures/src/main/resources/unsigned_long.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
692:x-pack/plugin/esql/qa/testFixtures/src/main/resources/where-like.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value
694:x-pack/plugin/esql/qa/testFixtures/src/main/resources/where-like.csv-spec:warningRegex:java.lang.IllegalArgumentException: single-value function encountered multi-value

I think removing it might require us changing all the existing warning messages.

@ioanatia ioanatia merged commit 1b66a2b into elastic:main Sep 26, 2025
34 checks passed
@ioanatia ioanatia deleted the fuse_stabilise_operators branch September 26, 2025 08:45
@ioanatia ioanatia mentioned this pull request Sep 25, 2025
16 tasks
szybia added a commit to szybia/elasticsearch that referenced this pull request Sep 26, 2025
…-dls

* upstream/main: (55 commits)
  Mute org.elasticsearch.upgrades.MatchOnlyTextRollingUpgradeIT testIndexing {upgradedNodes=1} elastic#135525
  Address es819 tsdb doc values format performance bug (elastic#135505)
  Remove obsolete --add-opens from JDK API extractor tool (elastic#135445)
  [CI] Fix MergeWithFailureIT (elastic#135447)
  Increase wait time in AdaptiveAllocationsScalerServiceTests (elastic#135510)
  ES|QL: Handle multi values in FUSE (elastic#135448)
  Mute org.elasticsearch.upgrades.SyntheticSourceRollingUpgradeIT testIndexing {upgradedNodes=1} elastic#135512
  Add trust configuration for cross cluster api keys (elastic#134893)
  ESQL: Fix flakiness in SessionUtilsTests (elastic#135375)
  Mute org.elasticsearch.upgrades.LogsdbIndexingRollingUpgradeIT testIndexing {upgradedNodes=1} elastic#135511
  Mute org.elasticsearch.upgrades.MatchOnlyTextRollingUpgradeIT testIndexing {upgradedNodes=2} elastic#135325
  Require all functions to provide examples (elastic#135094)
  Mute org.elasticsearch.upgrades.SyntheticSourceRollingUpgradeIT testIndexing {upgradedNodes=2} elastic#135344
  Mute org.elasticsearch.upgrades.TextRollingUpgradeIT testIndexing {upgradedNodes=1} elastic#135236
  Mute org.elasticsearch.upgrades.TextRollingUpgradeIT testIndexing {upgradedNodes=2} elastic#135238
  Mute org.elasticsearch.upgrades.LogsdbIndexingRollingUpgradeIT testIndexing {upgradedNodes=2} elastic#135327
  Mute org.elasticsearch.upgrades.MatchOnlyTextRollingUpgradeIT testIndexing {upgradedNodes=3} elastic#135324
  Mute org.elasticsearch.upgrades.StandardToLogsDbIndexModeRollingUpgradeIT testLogsIndexing {upgradedNodes=3} elastic#135315
  ESQL: Handle right hand side of Inline Stats coming optimized with LocalRelation shortcut (elastic#135011)
  Mute org.elasticsearch.upgrades.TextRollingUpgradeIT testIndexing {upgradedNodes=3} elastic#135237
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Search Relevance/ES|QL Search functionality in ES|QL Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants