Disallow unmapped_fields="load" with full-text search/MATCH by shmuelhanoch · Pull Request #143810 · elastic/elasticsearch

shmuelhanoch · 2026-03-08T21:58:02Z

Fail at analysis instead of returning empty results.
Add unit test testUnmappedFieldsLoadWithMatchOperatorFails in AnalyzerUnmappedTests.

Closes #141927

elasticsearchmachine · 2026-03-11T16:23:33Z

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

elasticsearchmachine · 2026-03-11T16:24:14Z

Hi @shmuelhanoch, I've created a changelog YAML for you.

mouhc1ne · 2026-03-11T16:43:29Z

...k/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerUnmappedTests.java

+     * Reproducer for #141927: with unmapped_fields=load, full-text search (match operator) must fail at analysis
+     * instead of returning empty results.
+     */
+    public void testUnmappedFieldsLoadWithMatchOperatorFails() {


Could we add few more tests here? Not necessarily more test functions, because we can verificationFailure(...) many times within the same function and pass many queries, I'd suspect there is a good number of interesting cases we want covered.

Done, I added one for each forbidden full-text function.

mouhc1ne · 2026-03-11T16:53:09Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

+                throw new VerificationException(
+                    List.of(
+                        Failure.fail(
+                            fullTextFunctions.get(0),


Looks like only the first function gets reported. But also, is finding a full-text function anywhere in query necessary and sufficient to reject it? Or do we want the reject only if they're applied to the unmapped field?

Only rejecting if applied to an unmapped field would certainly be better, but I wouldn't go there for now.

There's FTFs like KQL which can refer to an unmapped field inside the query string, as in WHERE KQL("author: Faulkner") (taken straight from our docs). author might be unmapped and it's not fully clear how we should behave then.

Change to report every full-text function in the plan.

alex-spies

Thanks @shmuelhanoch ! This gets the job done, but I'd like to increase test coverage and would also like to place the new verification into a slightly more appropriate place before merging. I added details below :)

alex-spies · 2026-03-12T16:37:06Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

        BitSet partialMetrics = new BitSet(FeatureMetric.values().length);
-        return verify(execute(plan), gatherPreAnalysisMetrics(plan, partialMetrics));
+        LogicalPlan analyzed = execute(plan);
+        if (context().unmappedResolution() == UnmappedResolution.LOAD) {


This is not the best place to add an additional verification. Analyzer#analyze is a pretty high-level method which has a simple workflow:

analyze

verify and gather usage data
We should keep it this way and not add low-level verifications to high-level execution logic.

We have 2 typical places for verifications that come to mind:

inside Verifier#verify (there's a section marked with Concrete verifications)

inside functions and commands themselves by implementing PostAnalysisPlanVerificationAware and PostAnalysisVerificationAware.

Now, neither of these places have access to the query settings. We could plug the query settings in there, and in fact it might become required anyway.

However, I think there's a simpler way.

We could add a verification step inside Verifier#verify which checks for the presence of full-text functions in case that the load setting had any effect on the query plan. More specifically, load will place FieldAttribute instances into the EsRelation which contain a PotentiallyUnmappedKeywordEsField. Such an attribute is thus marked as "will try to load from _source during execution". If we find any such field attribute in an EsRelation, we know that load was used, we do try to load some fields from _source, and we can thus fail the query if it contains disallowed functions or commands.

This also means that we should add positive tests for queries where load did not have any effect (because all fields in the query were, in fact, mapped), because full-text functions will not be forbidden in such queries.

Update: #144115 adds a new section inside Verifier#verify where we disallow not-yet allowed commands. I think we can wait on #144115 and move this PR's new validation next to it.

This is a little simpler than my original suggestion in that it will keep this PR's logic: disallow full-text functions if the query uses SET unmapped_fields="load". No need to check EsRelations for the presence of PotentiallyUnmappedKeywordEsFields in FieldAttributes.

OK, I moved the check to a Verifier#verify.

...k/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerUnmappedTests.java

alex-spies · 2026-03-12T16:52:21Z

...k/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerUnmappedTests.java

+     * Reproducer for #141927: with unmapped_fields=load, full-text search (match operator) must fail at analysis
+     * instead of returning empty results.


Appreciate the comment pointing to the issue!

super nit: I opened #144121 to explain that we plan to increase support for full-text functions. We could link to it as well so that future-us knows that the test will need changes in the future.

Linked #144121 in the test Javadoc!

alex-spies

Almost there! I have only few final comments and, once addressed, this certainly LGTM!

alex-spies · 2026-03-16T12:47:51Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java

        }

        if (unmappedResolution == UnmappedResolution.LOAD) {
            checkLoadModeDisallowedCommands(plan, failures);


Here's the existing check by Andrei.

I suggest we add another helper method checkLoadModeDisallowedFunctions and call it here.

OK, extracted into checkLoadModeDisallowedFunctions here

alex-spies · 2026-03-16T12:48:37Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java

+        // Disallow full-text search when unmapped_fields=load. We do not restrict to "only when the FTF
+        // is applied to an unmapped field" because FTFs like KQL can reference unmapped fields inside the
+        // query string (e.g. KQL("author: Faulkner")), and the desired behavior there is unclear.


++, thanks for commenting with the reason for the code here. I think this will help future us (and others!) understand the choices we made.

alex-spies · 2026-03-16T12:50:25Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java

+            List<FullTextFunction> fullTextFunctions = new ArrayList<>();
+            plan.forEachExpressionDown(FullTextFunction.class, fullTextFunctions::add);
+            if (fullTextFunctions.isEmpty() == false) {
+                String message = "unmapped_fields=\"load\" is not allowed with full-text search (MATCH, match operator `:`, "


Rather than mentioning MATCH specifically in the error message, why don't we tell the user which full-text function they used?

Ideally, our tests should also assert that the right full text function was mentioned in the error message.

When reporting user input in error messages, we typically enclose it in brackets, like so:

unmapped_fields="load" does not support full-text search function [KQL]; use "fail" or "nullify"

except that the KQL needs to be replaced by the actual function that was used.

Sure, I fixed this to show the actual function name and check it in the tests

alex-spies

LGTM! Thanks @shmuelhanoch !

…lastic#141927) Fail at analysis instead of returning empty results. Add unit test testUnmappedFieldsLoadWithMatchOperatorFails in AnalyzerUnmappedTests.

- Move unmapped_fields=load + full-text verification from Analyzer to Verifier (new verify(plan, partialMetrics, unmappedResolution) overload). - Report all full-text function sites in the plan, not only the first. - Test each forbidden full-text function: match op, match, match_phrase, multi_match, qstr, kql, knn (capability-guarded where needed). - Add verificationFailureWithMapping helper for KNN test; Javadoc elastic#144121.

- Extract full-text check into checkLoadModeDisallowedFunctions helper - Error message shows actual function name in brackets (e.g. [KQL], [MATCH]) - Tests assert correct function name in each failure message

…143810) * ESQL: fail when unmapped_fields=load is used with full-text search (elastic#141927) Fail at analysis instead of returning empty results. Add unit test testUnmappedFieldsLoadWithMatchOperatorFails in AnalyzerUnmappedTests. * Review fixes: - Move unmapped_fields=load + full-text verification from Analyzer to Verifier (new verify(plan, partialMetrics, unmappedResolution) overload). - Report all full-text function sites in the plan, not only the first. - Test each forbidden full-text function: match op, match, match_phrase, multi_match, qstr, kql, knn (capability-guarded where needed). - Add verificationFailureWithMapping helper for KNN test; Javadoc elastic#144121. * Fix Verifier Javadoc and AnalyzerUnmappedTests for CI * More review fixes: - Extract full-text check into checkLoadModeDisallowedFunctions helper - Error message shows actual function name in brackets (e.g. [KQL], [MATCH]) - Tests assert correct function name in each failure message

elasticsearchmachine added the v9.4.0 label Mar 8, 2026

shmuelhanoch force-pushed the 141927-esql-disallow-unmapped-load-with-match branch from 80a166b to 324bc55 Compare March 9, 2026 07:02

shmuelhanoch marked this pull request as ready for review March 11, 2026 16:18

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Mar 11, 2026

shmuelhanoch requested a review from alex-spies March 11, 2026 16:18

shmuelhanoch force-pushed the 141927-esql-disallow-unmapped-load-with-match branch from 324bc55 to db073ff Compare March 11, 2026 16:20

shmuelhanoch added the :Analytics/ES|QL AKA ESQL label Mar 11, 2026

elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed needs:triage Requires assignment of a team area label labels Mar 11, 2026

shmuelhanoch added >bug needs:triage Requires assignment of a team area label and removed Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Mar 11, 2026

elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed needs:triage Requires assignment of a team area label labels Mar 11, 2026

shmuelhanoch added >non-issue and removed >bug labels Mar 11, 2026

shmuelhanoch force-pushed the 141927-esql-disallow-unmapped-load-with-match branch from 7c13171 to 17c80e0 Compare March 11, 2026 16:47

mouhc1ne reviewed Mar 11, 2026

View reviewed changes

alex-spies requested changes Mar 12, 2026

View reviewed changes

shmuelhanoch force-pushed the 141927-esql-disallow-unmapped-load-with-match branch 2 times, most recently from c81800e to 2990547 Compare March 16, 2026 03:22

alex-spies reviewed Mar 16, 2026

View reviewed changes

shmuelhanoch requested review from alex-spies and mouhc1ne March 16, 2026 14:17

shmuelhanoch force-pushed the 141927-esql-disallow-unmapped-load-with-match branch 2 times, most recently from 74504ad to 19c271f Compare March 17, 2026 07:05

alex-spies approved these changes Mar 17, 2026

View reviewed changes

ESQL: fail when unmapped_fields=load is used with full-text search (e…

cfce064

…lastic#141927) Fail at analysis instead of returning empty results. Add unit test testUnmappedFieldsLoadWithMatchOperatorFails in AnalyzerUnmappedTests.

shmuelhanoch added 3 commits March 17, 2026 19:39

Fix Verifier Javadoc and AnalyzerUnmappedTests for CI

e4914e8

More review fixes:

57aaf9b

- Extract full-text check into checkLoadModeDisallowedFunctions helper - Error message shows actual function name in brackets (e.g. [KQL], [MATCH]) - Tests assert correct function name in each failure message

shmuelhanoch force-pushed the 141927-esql-disallow-unmapped-load-with-match branch from 19c271f to 57aaf9b Compare March 17, 2026 17:39

shmuelhanoch enabled auto-merge (squash) March 17, 2026 17:39

mouhc1ne approved these changes Mar 17, 2026

View reviewed changes

shmuelhanoch merged commit b52f511 into elastic:main Mar 17, 2026
36 checks passed

alex-spies mentioned this pull request Mar 18, 2026

Disallow unmapped_fields=load with partial non-KEYWORD #144109

Merged

		* Reproducer for #141927: with unmapped_fields=load, full-text search (match operator) must fail at analysis
		* instead of returning empty results.

Conversation

shmuelhanoch commented Mar 8, 2026

Uh oh!

elasticsearchmachine commented Mar 11, 2026

Uh oh!

elasticsearchmachine commented Mar 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-spies left a comment

Choose a reason for hiding this comment

Uh oh!

alex-spies Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-spies Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-spies left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-spies left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alex-spies Mar 12, 2026 •

edited

Loading

alex-spies Mar 13, 2026 •

edited

Loading