ESQL: introduce support for mapping-unavailable fields (Fork from #139417)#140463
Conversation
This will allow re-evaluating the output past a RENAME/DROP/KEEP, once an unmapping field is injected.
| var outputNames = eval.outputSet().names(); | ||
| var evalRefNames = eval.references().names(); | ||
| for (Alias a : nullAliases) { | ||
| if (outputNames.contains(a.name()) == false) { |
There was a problem hiding this comment.
We only check the output of the existing eval - not the output of the leaf below it. This can some times put the plan into an inconsistent state during resolution, where properly resolved references point to a field that was shadowed. It's probably fine, because that should only happen when the plan was invalid to begin with.
Example: Consider a field foo that exists in the index
SET unmapped_fields=\"nullify\"; from test | where foo > 1 | drop foo | where foo > 2
[2026-01-12T14:21:39,991][TRACE][o.e.x.e.a.A.changes ] [runTask-0] Rule rules.ResolveUnmapped applied with change
Filter[?foo > 2[INTEGER]] = Filter[?foo > 2[INTEGER]]
\_ResolvingProject[org.elasticsearch.xpack.esql.analysis.Analyzer$ResolveRefs$$Lambda/0x000000002b759490@5c6fd1e4,[bar{f}#130]] = \_ResolvingProject[org.elasticsearch.xpack.esql.analysis.Analyzer$ResolveRefs$$Lambda/0x000000002b759490@5c6fd1e4,[bar{f}#130]]
\_Filter[foo{f}#131 > 1[INTEGER]] = \_Filter[foo{f}#131 > 1[INTEGER]]
\_EsRelation[test][bar{f}#130, foo{f}#131] ! \_Eval[[null[NULL] AS foo#132]]
! \_EsRelation[test][bar{f}#130, foo{f}#131]
The first filter where foo > 1 first resolves correctly, but then ResolveUnmapped finds that foo is missing downstream - and adds an Eval that shadows the correctly resolved foo, making the filter use a missing column.
This could lead to wrong error message if we start checking for plan consistency during analyzer runs already - so it's best to add a test.
Added a follow-up item to #138888.
There was a problem hiding this comment.
Initial test existing (testFailEvalAfterDrop), but introducing also a new one (testFailFilterAfterDrop).
| ); | ||
| // insert an Eval on top of those LeafPlan that are children of n-ary plans (could happen with UnionAll) | ||
| transformed = transformed.transformUp( | ||
| n -> n instanceof UnaryPlan == false && n instanceof LeafPlan == false, |
There was a problem hiding this comment.
nullify adds wrong null fields to the lookup index when a field is missing after the join. This will need fixing, even though it seems not to actually trigger a visible bug at the moment. I think we want to exclude the right hand side of joins, as that never corresponds to a FROM command.
SET unmapped_fields=\"nullify\"; FROM employees| EVAL language_code = languages| LOOKUP JOIN languages_lookup ON language_code | limit 1 | where missing::integer > 1
Notice how we add an eval with nulls to the lookup index (even though we cannot actually execute this eval on the lookup side!)
[2026-01-12T14:36:17,805][TRACE][o.e.x.e.a.A.changes ] [runTask-0] Rule rules.ResolveUnmapped applied with change
Filter[TOINTEGER(?missing) > 1[INTEGER]] = Filter[TOINTEGER(?missing) > 1[INTEGER]]
\_Limit[1[INTEGER],false,false] = \_Limit[1[INTEGER],false,false]
\_LookupJoin[LEFT,[language_code{r}#233],[language_code{f}#259],false,null] = \_LookupJoin[LEFT,[language_code{r}#233],[language_code{f}#259],false,null]
|_Eval[[languages{f}#238 AS language_code#233]] ! |_Eval[[languages{f}#238 AS language_code#233, null[NULL] AS missing#261]]
| \_EsRelation[employees][avg_worked_seconds{f}#236, birth_date{f}#243, emp_n..] = | \_EsRelation[employees][avg_worked_seconds{f}#236, birth_date{f}#243, emp_n..]
\_EsRelation[languages_lookup][LOOKUP][language_code{f}#259, language_name{f}#260] ! \_Eval[[null[NULL] AS missing#261]]
! \_EsRelation[languages_lookup][LOOKUP][language_code{f}#259, language_name{f}#260]
Added a follow-up item to #138888.
| // TODO: would an alternative to this be to drop the current Fork and have ResolveRefs#resolveFork re-resolve it. We might need | ||
| // some plan delimiters/markers to make it unequivocal which nodes belong to "make Fork work" - like (Limit-Project[-Eval])s - and | ||
| // which don't. | ||
| private static Fork patchFork(Fork fork, List<Attribute> aliasAttributes) { |
There was a problem hiding this comment.
What's required to patch FORK is currently quite complex. I think we should see if it can be simplified.
We simplified Project via ResolvingProject; previously, this required workarounds because a Project had fixed output attributes rather than computing them from its inputs. It kinda looks like Fork has similar problems, and it's quite a bit of a dance to get it to work here.
Added a follow-up item to #138888.
There was a problem hiding this comment.
| * {@link Analyzer.ResolveRefs} to attempt again to wire them to the newly added aliases. That's what this method does. | ||
| */ | ||
| private static LogicalPlan refreshUnresolved(LogicalPlan plan, List<UnresolvedAttribute> unresolved) { | ||
| return plan.transformExpressionsOnlyUp(UnresolvedAttribute.class, ua -> { |
There was a problem hiding this comment.
I think we shouldn't have to transform up here. If there are children with attributes that ResolveRefs deemed unresolvable, ResolveUnmapped shouldn't yet be looking at the current plan, but still be doing its work on the unresolved children.
In fact, maybe we shouldn't mark fields as unresolveable until we're in the clean-up step. Being unresolvable just cannot be determined in a single run of ResolveRefs in case of nullify or load.
Added a follow-up item to #138888.
There was a problem hiding this comment.
maybe we shouldn't mark fields as unresolveable until we're in the clean-up step
I think this might be an optimisation that can be dropped, but didn't look into it, if that really is the case.
I think we shouldn't have to transform up here.
The comment may read as "this code is wrong, it shouldn't do that". But I think you mean: "this, as well as handling UnresolvedAttribute_s, can potentially be refactored". Making the distinction because with the existing optimisation, the new feature / code has to do a refresh.
Added a follow-up item to #138888.
I'll leave it there, but it might be a follow-up to #138888 itself; i.e.: we could make the code pre-unmapped fields simpler by removing that pre-existing optimization.
| * @return A plan having all nodes recreated (no properties changed, otherwise). This is needed to clear internal, lazy-eval'd and | ||
| * cached state, such as the output. The rule inserts new attributes in the plan, so the output of all the nodes downstream these | ||
| * insertions need be recomputed. | ||
| */ | ||
| private static LogicalPlan refreshChildren(LogicalPlan plan) { |
There was a problem hiding this comment.
IMHO this is a smell. If a plan node's output depends on its children, no re-computation should be required - changing the children should not require additional steps for the .output() method to return correct results.
Added a follow-up item to #138888.
💔 Backport failed
You can use sqren/backport to manually backport by running |
This introduces support for mapping-unavailable fields (present and not mapped or just missing). The behaviour is controlled through a new SET setting unmapped_fields, which can take the values "FAIL", "NULLIFY", "LOAD". An optional field behaves just like a "normal", mapped field, with regards to how it flows through the commands chain: it can be simply used in the commands, as if present in the source, but can no longer be referenced once dropped - explicitly, with DROP, or not selected by a KEEP, or RENAME that doesn't reference it -, or past a STATS reduction. However, unlike a mapped field, if it's not reference at all, it won't show up in the output of a simple FROM index. Currently, the schema difference between nullified fields and the loaded ones is in the type: nullified ones are of data type NULL, while the loaded ones are KEYWORD. The implementation difference w.r.t. logical plan building is that the nullified fields are created as null value aliasing on top of the data source, while the loaded one are pushed as extractors into the source (this leverages the INSIST work). The partially mapped fields are also covered: when the setting is "load", these fields will be extracted from those indices that have the field, but isn't mapped. In case there's a conflict between the loaded KEYWORD field and the mapped type in the fields that have this field mapped, an explicit conversion is needed, just like with union types. Related: elastic#138888 (cherry picked from commit ff745c0) # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/AnalyzerContext.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/QuerySettings.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/promql/PromqlLogicalPlanOptimizerTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/SetParserTests.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…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) ...
| public void testFailDropWithNonMatchingStar() { | ||
| var query = """ | ||
| FROM test | ||
| | DROP does_not_exist_field* | ||
| """; | ||
| var failure = "No matches found for pattern [does_not_exist_field*]"; | ||
| verificationFailure(setUnmappedNullify(query), failure); | ||
| verificationFailure(setUnmappedLoad(query), failure); |
There was a problem hiding this comment.
I think this is a problem. If I have a does_not_exist_field in the test index, but it's e.g. rolled over and previous physical indices were missing this field - well, my query will fail if I point them at the previous indices because they're missing the field.
The consequence is that DROP probably should ignore missing fields altogether in case of SET unmapped_fields="nullify".
There are more tests like this below that should be revisitied, like testFailDropWithMatchingAndNonMatchingStar.
Added to #138888
There was a problem hiding this comment.
I eventually agree. I opted for keeping it in sync with the non-mapped_fields behaviour, but I think the feature's intention warrants the exception. We should make sure we also update DROP's docs.
| var plan = analyzeStatement(setUnmappedNullify(""" | ||
| FROM test | ||
| | STATS s = SUM(does_not_exist1) + d2 BY d2 = does_not_exist2, emp_no | ||
| """)); |
There was a problem hiding this comment.
This wrongly adds an EVAL d2 = null in the beginning. This leads to inconsistent plans.
Example:
SET unmapped_fields=\"nullify\"; from test | where foo == 1 | stats count(*) by foo = does_not_exist
Even if the field foo exists, ResolveUnmapped still adds an EVAL foo = null, shadowing the existing foo field!
[2026-01-13T19:22:03,525][TRACE][o.e.x.e.a.A.changes ] [runTask-0] Rule rules.ResolveUnmapped applied with change
Aggregate[[?does_not_exist AS foo#52],[COUNT(*[KEYWORD],true[BOOLEAN],PT0S[TIME_DURATION]) AS count(*)#53, ?foo]] = Aggregate[[?does_not_exist AS foo#52],[COUNT(*[KEYWORD],true[BOOLEAN],PT0S[TIME_DURATION]) AS count(*)#53, ?foo]]
\_Filter[foo{f}#55 == 1[INTEGER]] = \_Filter[foo{f}#55 == 1[INTEGER]]
\_EsRelation[test][foo{f}#55] ! \_Eval[[null[NULL] AS does_not_exist#56, null[NULL] AS foo#57]]
! \_EsRelation[test][foo{f}#55]
Added to #138888
There was a problem hiding this comment.
Fixing in #141340 (testStatsAggAndAliasedGroup, testStatsAggAndAliasedShadowingGroup, testStatsAggAndAliasedShadowingGroupOverExpression)
| * | \_Eval[[null[NULL] AS does_not_exist1#13, null[NULL] AS does_not_exist2#30]] | ||
| * | \_EsRelation[languages][language_code{f}#7, language_name{f}#8] | ||
| * \_Limit[1000[INTEGER],false,false] | ||
| * \_EsqlProject[[language_code{r}#21, language_name{r}#22, does_not_exist1{r}#15, @timestamp{f}#9, client_ip{f}#10, | ||
| * event_duration{f}#11, message{f}#12, does_not_exist2{r}#32, $$does_not_exist2$converted_to$long{r}#35]] | ||
| * \_Eval[[TOLONG(does_not_exist2{r}#32) AS $$does_not_exist2$converted_to$long#35]] | ||
| * \_Eval[[null[INTEGER] AS language_code#21, null[KEYWORD] AS language_name#22]] | ||
| * \_Subquery[] | ||
| * \_Filter[TODOUBLE(does_not_exist1{r}#15) > 10.0[DOUBLE]] | ||
| * \_Eval[[null[NULL] AS does_not_exist1#15, null[NULL] AS does_not_exist2#30]] |
There was a problem hiding this comment.
This doesn't look right: the does_not_exist2 has the same name id in both branches. Branches should not share name ids in subqueries - the two columns have different meaning in general, even if they'd be to refer to the same existing field in both branches.
Added to #138888
| * \_EsqlProject[[_meta_field{r}#30, emp_no{r}#31, first_name{r}#32, gender{r}#33, hire_date{r}#34, job{r}#35, job.raw{r}#36, | ||
| * languages{r}#37, last_name{r}#38, long_noidx{r}#39, salary{r}#40, c{r}#4, does_not_exist{r}#55]] | ||
| * \_Eval[[null[NULL] AS does_not_exist#56]] |
There was a problem hiding this comment.
This is quite inconsistent. The Projection projects on does_not_exist with id 55 - but the Evals below create columns with name does_not_exist and different name ids.
This also happens in other test cases with subqueries below.
Added to #138888
#140526) * ESQL: introduce support for mapping-unavailable fields (#140463) This introduces support for mapping-unavailable fields (present and not mapped or just missing). The behaviour is controlled through a new SET setting unmapped_fields, which can take the values "FAIL", "NULLIFY", "LOAD". An optional field behaves just like a "normal", mapped field, with regards to how it flows through the commands chain: it can be simply used in the commands, as if present in the source, but can no longer be referenced once dropped - explicitly, with DROP, or not selected by a KEEP, or RENAME that doesn't reference it -, or past a STATS reduction. However, unlike a mapped field, if it's not reference at all, it won't show up in the output of a simple FROM index. Currently, the schema difference between nullified fields and the loaded ones is in the type: nullified ones are of data type NULL, while the loaded ones are KEYWORD. The implementation difference w.r.t. logical plan building is that the nullified fields are created as null value aliasing on top of the data source, while the loaded one are pushed as extractors into the source (this leverages the INSIST work). The partially mapped fields are also covered: when the setting is "load", these fields will be extracted from those indices that have the field, but isn't mapped. In case there's a conflict between the loaded KEYWORD field and the mapped type in the fields that have this field mapped, an explicit conversion is needed, just like with union types. Related: #138888 (cherry picked from commit ff745c0) # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/AnalyzerContext.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/QuerySettings.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/promql/PromqlLogicalPlanOptimizerTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/SetParserTests.java * Revert transition EsqlProject->Project That refactoring was not backported to 9.3. * Spotless * Fix compilation error The analyzer code was adapted for TimeSeriesAggregate becoming TimestampAware in #140270, but that was not backported to 9.3. Adapt to the old version of the code. * Fix tests --------- Co-authored-by: Gal Lalouche <gal.lalouche@elastic.co>
| assumeTrue("Requires OPTIONAL_FIELDS", EsqlCapabilities.Cap.OPTIONAL_FIELDS.isEnabled()); | ||
| return "SET unmapped_fields=\"nullify\"; " + query; | ||
| } | ||
|
|
||
| private static String setUnmappedLoad(String query) { | ||
| assumeTrue("Requires OPTIONAL_FIELDS", EsqlCapabilities.Cap.OPTIONAL_FIELDS.isEnabled()); |
There was a problem hiding this comment.
The assumeTrues being inside the setUnmapped... methods is not ideal because we will release nullify earlier than load. Because tests often have calls to both setUnmappedNullify and setUnmappedLoad, this means that in release-test runs, the test will be considered skipped just because we don't have load enabled in release builds.
Let's split up the tests for load and nullify, so that load tests getting skipped doesn't affect any tests for nullify. I.e., a test should have either setUnmappedNullify or setUnmappedLoad, but not both.
Added to #138888
…astic#140528) This PR removes the snapshot protection of FAIL and NULLIFY options for unmapped fields (only LOAD remains protected under snapshot). Follow up to elastic#140463. Related: elastic#138888.
…ew (#140528) (#140657) * ESQL: Enable nullify and fail unmapped resolution in tech-preview (#140528) This PR removes the snapshot protection of FAIL and NULLIFY options for unmapped fields (only LOAD remains protected under snapshot). Follow up to #140463. Related: #138888. * Fix statsAggs tests --------- Co-authored-by: Alexander Spies <alexander.spies@elastic.co>
This introduces support for mapping-unavailable fields (present and not mapped or just missing). The behaviour is controlled through a new SET setting unmapped_fields, which can take the values "FAIL", "NULLIFY", "LOAD". An optional field behaves just like a "normal", mapped field, with regards to how it flows through the commands chain: it can be simply used in the commands, as if present in the source, but can no longer be referenced once dropped - explicitly, with DROP, or not selected by a KEEP, or RENAME that doesn't reference it -, or past a STATS reduction. However, unlike a mapped field, if it's not reference at all, it won't show up in the output of a simple FROM index. Currently, the schema difference between nullified fields and the loaded ones is in the type: nullified ones are of data type NULL, while the loaded ones are KEYWORD. The implementation difference w.r.t. logical plan building is that the nullified fields are created as null value aliasing on top of the data source, while the loaded one are pushed as extractors into the source (this leverages the INSIST work). The partially mapped fields are also covered: when the setting is "load", these fields will be extracted from those indices that have the field, but isn't mapped. In case there's a conflict between the loaded KEYWORD field and the mapped type in the fields that have this field mapped, an explicit conversion is needed, just like with union types. Related: elastic#138888
…astic#140528) This PR removes the snapshot protection of FAIL and NULLIFY options for unmapped fields (only LOAD remains protected under snapshot). Follow up to elastic#140463. Related: elastic#138888.
4 participants
This a PR fork of #139417, with the empahsis of getting the
NULLIFYoption ready. Main changes: