Skip to content

ESQL: Unique ids when replacing missing fields#125656

Closed
alex-spies wants to merge 17 commits intoelastic:mainfrom
alex-spies:unique-ids-when-replacing-missing-fields
Closed

ESQL: Unique ids when replacing missing fields#125656
alex-spies wants to merge 17 commits intoelastic:mainfrom
alex-spies:unique-ids-when-replacing-missing-fields

Conversation

@alex-spies
Copy link
Contributor

@alex-spies alex-spies commented Mar 26, 2025

Fix #121754

Branched off #125462, but with a different solution.

Requires more local logical plan optimizer tests before this should be merged.
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Mar 26, 2025

Hi @alex-spies, I've created a changelog YAML for you.
@alex-spies
Just create new ids always when replacing
895d8a2
@alex-spies
Simplify
6ae7223 
Don't try to optimize for multiple occurrences of the same, missing
field attr in a projection. That can't occur, anyway, as it would breach
our invariants.
@alex-spies
Minimize diff to previous approach
76640ed
@alex-spies
Be more selective when to update the plan's exprs
702aaf7
@alex-spies alex-spies mentioned this pull request Mar 27, 2025
Merged
alex-spies
alex-spies commented Mar 27, 2025
View reviewed changes
.../org/elasticsearch/xpack/esql/optimizer/rules/logical/local/ReplaceMissingFieldWithNull.java
}
} else if (plan instanceof Project) {
// Only create null literals for newly encountered missing fields
plan = resolveAlreadyReplacedAttributes(plan, fieldAttrReplacedBy);
Copy link
Contributor Author

@alex-spies alex-spies Mar 27, 2025

Choose a reason for hiding this comment

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

This is problematic, see #110373
@alex-spies
Copy link
Contributor Author

alex-spies commented Mar 27, 2025

The present approach could lead to class cast exceptions; fixing this introduces incidental complexity. Closing.
@alex-spies alex-spies closed this Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >bug v9.1.0

3 participants

@alex-spies @elasticsearchmachine @fang-xing-esql