Skip to content

Fix lookup join nullify#142863

Closed
idegtiarenko wants to merge 1 commit intoelastic:mainfrom
idegtiarenko:fix_lookup_join_nullify
Closed

Fix lookup join nullify#142863
idegtiarenko wants to merge 1 commit intoelastic:mainfrom
idegtiarenko:fix_lookup_join_nullify

Conversation

@idegtiarenko
Copy link
Copy Markdown
Contributor

@idegtiarenko idegtiarenko commented Feb 23, 2026

This change fixes lookup queries with unknown fields executed with SET unmapped_fields="nullify";.
Such queries produced the following plan:

Optimized logicalPlan plan:
    Project[[language_code{f}#8, language_name{f}#11, foobar{r}#12]]
    \_Limit[1000[INTEGER],true,false]
      \_Join[LEFT,[language_code{f}#8],[language_code{f}#10],null]
        |_Eval[[null[NULL] AS foobar#12]]
        | \_Limit[1000[INTEGER],false,false]
        |   \_EsRelation[languages][language_code{f}#8, language_name{f}#9]
        \_Eval[[null[NULL] AS foobar#12]]                                              <------------------------------
          \_EsRelation[languages_lookup][LOOKUP][language_code{f}#10, language_name{f}#11]

and could not be mapped to a physical plan.

The fix is to avoid inserting eval on top of lookup sources.
Alternatively we should consider setting eval on top of Join instead of individual EsRelations.

Fixes: #141978

@idegtiarenko idegtiarenko added >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.4.0 labels Feb 23, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Feb 23, 2026

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

@idegtiarenko
Copy link
Copy Markdown
Contributor Author

idegtiarenko commented Feb 25, 2026 •
edited
Loading

The same issue might be addressed by #141340

alex-spies
alex-spies reviewed Mar 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies left a comment

Choose a reason for hiding this comment

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

Thanks @idegtiarenko !

Looks correct. As discussed, I prefer getting Bogdan's PR in (because I'm already fighting CI over there), which will make this obsolete, I'm afraid :/ I'll add your test, though, and will attribute to you if git permits!

x-pack/plugin/esql/qa/testFixtures/src/main/resources/unmapped-load.csv-spec
Comment on lines +101 to +102
required_capability: optional_fields_nullify_tech_preview
required_capability: join_lookup_v12
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies Mar 4, 2026

Choose a reason for hiding this comment

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

Probably failing bwc tests due to no new capability required here.

x-pack/plugin/esql/qa/testFixtures/src/main/resources/unmapped-load.csv-spec
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies Mar 4, 2026

Choose a reason for hiding this comment

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

The behavior is probably the correct one, but we'll disallow joins with load for now until we're certain that we always want to load from the main index (I think we do). That's #142367

...k/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/rules/ResolveUnmapped.java
Comment on lines +214 to +217
if (child instanceof EsRelation esr && esr.indexMode() == IndexMode.LOOKUP) {
newChildren.add(child);
continue;
}
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies Mar 4, 2026

Choose a reason for hiding this comment

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

Essentially the same fix as in Bogdan's PR https://github.com/elastic/elasticsearch/pull/141340/changes#diff-b135751577c8077e6aebc28a662beca9f604826869d1d368c60fc8f1bb0a2e4bR212-R219.

x-pack/plugin/esql/qa/testFixtures/src/main/resources/unmapped-nullify.csv-spec
Comment on lines +477 to +480
SET unmapped_fields="nullify"\;
FROM languages
| LOOKUP JOIN languages_lookup ON language_code
| KEEP language_code, language_name, foobar
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies Mar 4, 2026

Choose a reason for hiding this comment

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

👍

alex-spies added a commit to bpintea/elasticsearch that referenced this pull request Mar 4, 2026
@alex-spies @idegtiarenko
Add reproducer test for join issue
222b7c3 
Extracted from elastic#142863

Co-authored-by: Ievgen Degtiarenko <ievgen.degtiarenko@elastic.co>
alex-spies
alex-spies reviewed Mar 4, 2026
View reviewed changes
x-pack/plugin/esql/qa/testFixtures/src/main/resources/unmapped-nullify.csv-spec
Copy link
Copy Markdown
Contributor

@alex-spies alex-spies Mar 4, 2026

Choose a reason for hiding this comment

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

Absorbed into Bogdan's PR: 222b7c3

@idegtiarenko
Copy link
Copy Markdown
Contributor Author

idegtiarenko commented Mar 5, 2026

Closing in favor of 222b7c3

@idegtiarenko idegtiarenko deleted the fix_lookup_join_nullify branch March 5, 2026 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alex-spies alex-spies alex-spies left review comments

Assignees

No one assigned

Labels

:Analytics/ES|QL AKA ESQL >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ESQL: unmapped_fields="nullify" leads to LOOKUP JOIN Mapper failure

3 participants

@idegtiarenko @elasticsearchmachine @alex-spies