Merge remote-tracking branch 'elastic/8.19' into backport/8.19/pr-130849

julian-elastic · julian-elastic · commit 2b323265d254 · 2025-07-11T12:37:17.000-04:00
diff --git a/docs/changelog/130914.yaml b/docs/changelog/130914.yaml
@@ -0,0 +1,6 @@
+pr: 130914
+summary: Fix LIMIT NPE with null value
+area: ES|QL
+type: bug
+issues:
+ - 130908
diff --git a/docs/reference/esql/esql-limitations.asciidoc b/docs/reference/esql/esql-limitations.asciidoc
@@ -302,3 +302,17 @@ the <<esql-mv-functions,multivalue functions>>.
 === Kibana limitations
 
 include::esql-kibana.asciidoc[tag=limitations]
+
+[discrete]
+[[esql-known-issues]]
+== Known issues
+
+A bug in the ES|QL STATS command may yield incorrect results. The bug only happens in very specific cases that follow this pattern: `STATS ... BY keyword1, keyword2`, i.e. the command must have exactly two grouping fields, both keywords, where the first field has high cardinality (more than 65k distinct values).
+
+The bug is described in detail in [this issue](https://github.com/elastic/elasticsearch/issues/130644).
+The problem was introduced in 8.16.0 and [fixed](https://github.com/elastic/elasticsearch/pull/130705) in 8.17.9, 8.18.7.
+
+Possible workarounds include:
+  * switching the order of the grouping keys (eg. `STATS ... BY keyword2, keyword1`, if the `keyword2` has a lower cardinality)
+  * reducing the grouping key cardinality, by filtering out values before STATS
+
diff --git a/docs/reference/release-notes/8.16.0.asciidoc b/docs/reference/release-notes/8.16.0.asciidoc
@@ -517,3 +517,13 @@ Snapshot/Restore::
 {esql}::
 
 * Some valid queries using an `ENRICH` command can fail when a match field is used that is absent from some indices or shards, either with a 500 status code due to `NullPointerException` or `ClassCastException` or with a 400 status code and `IllegalArgumentException`. This is fixed in {es-pull}126187[#126187].
+
+* A bug in the ES|QL STATS command may yield incorrect results. The bug only happens in very specific cases that follow this pattern: `STATS ... BY keyword1, keyword2`, i.e. the command must have exactly two grouping fields, both keywords, where the first field has high cardinality (more than 65k distinct values).
+
+  The bug is described in detail in [this issue](https://github.com/elastic/elasticsearch/issues/130644).
+  The problem was introduced in 8.16.0 and [fixed](https://github.com/elastic/elasticsearch/pull/130705) in 8.17.9, 8.18.7.
+
+  Possible workarounds include:
+  * switching the order of the grouping keys (eg. `STATS ... BY keyword2, keyword1`, if the `keyword2` has a lower cardinality)
+  * reducing the grouping key cardinality, by filtering out values before STATS
+
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/search_mvt.json b/rest-api-spec/src/main/resources/rest-api-spec/api/search_mvt.json
@@ -73,6 +73,15 @@
         "description":"Determines the geometry type for features in the aggs layer.",
         "default":"grid"
       },
+      "grid_agg":{
+        "type":"enum",
+        "options":[
+          "geotile",
+          "geohex"
+        ],
+        "description":"Aggregation used to create a grid for `field`.",
+        "default":"geotile"
+      },
       "size":{
         "type":"int",
         "description":"Maximum number of features to return in the hits layer. Accepts 0-10000.",
diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle
@@ -230,6 +230,7 @@ tasks.named("yamlRestTestV7CompatTransform").configure({ task ->
   task.skipTest("esql/191_lookup_join_on_datastreams/data streams not supported in LOOKUP JOIN", "Added support for aliases in JOINs")
   task.skipTest("esql/190_lookup_join/non-lookup index", "Error message changed")
   task.skipTest("esql/192_lookup_join_on_aliases/alias-pattern-multiple", "Error message changed")
+  task.skipTest("esql/10_basic/Test wrong LIMIT parameter", "Error message changed")
 })
 
 
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneCountOperator.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneCountOperator.java
@@ -56,7 +56,7 @@ public Factory(
                 taskConcurrency,
                 limit,
                 false,
-                ScoreMode.COMPLETE_NO_SCORES
+                shardContext -> ScoreMode.COMPLETE_NO_SCORES
             );
         }
 
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMaxFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMaxFactory.java
@@ -128,7 +128,7 @@ public LuceneMaxFactory(
             taskConcurrency,
             limit,
             false,
-            ScoreMode.COMPLETE_NO_SCORES
+            shardContext -> ScoreMode.COMPLETE_NO_SCORES
         );
         this.fieldName = fieldName;
         this.numberType = numberType;
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneMinFactory.java
@@ -128,7 +128,7 @@ public LuceneMinFactory(
             taskConcurrency,
             limit,
             false,
-            ScoreMode.COMPLETE_NO_SCORES
+            shardContext -> ScoreMode.COMPLETE_NO_SCORES
         );
         this.fieldName = fieldName;
         this.numberType = numberType;
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneOperator.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneOperator.java
@@ -101,11 +101,18 @@ protected Factory(
             int taskConcurrency,
             int limit,
             boolean needsScore,
-            ScoreMode scoreMode
+            Function<ShardContext, ScoreMode> scoreModeFunction
         ) {
             this.limit = limit;
             this.dataPartitioning = dataPartitioning;
-            this.sliceQueue = LuceneSliceQueue.create(contexts, queryFunction, dataPartitioning, autoStrategy, taskConcurrency, scoreMode);
+            this.sliceQueue = LuceneSliceQueue.create(
+                contexts,
+                queryFunction,
+                dataPartitioning,
+                autoStrategy,
+                taskConcurrency,
+                scoreModeFunction
+            );
             this.taskConcurrency = Math.min(sliceQueue.totalSlices(), taskConcurrency);
             this.needsScore = needsScore;
         }
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneSliceQueue.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneSliceQueue.java
@@ -107,12 +107,14 @@ public static LuceneSliceQueue create(
         DataPartitioning dataPartitioning,
         Function<Query, PartitioningStrategy> autoStrategy,
         int taskConcurrency,
-        ScoreMode scoreMode
+        Function<ShardContext, ScoreMode> scoreModeFunction
     ) {
         List<LuceneSlice> slices = new ArrayList<>();
         Map<String, PartitioningStrategy> partitioningStrategies = new HashMap<>(contexts.size());
+
         for (ShardContext ctx : contexts) {
             for (QueryAndTags queryAndExtra : queryFunction.apply(ctx)) {
+                var scoreMode = scoreModeFunction.apply(ctx);
                 Query query = queryAndExtra.query;
                 query = scoreMode.needsScores() ? query : new ConstantScoreQuery(query);
                 /*
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneSourceOperator.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneSourceOperator.java
@@ -80,7 +80,7 @@ public Factory(
                 taskConcurrency,
                 limit,
                 needsScore,
-                needsScore ? COMPLETE : COMPLETE_NO_SCORES
+                shardContext -> needsScore ? COMPLETE : COMPLETE_NO_SCORES
             );
             this.maxPageSize = maxPageSize;
             // TODO: use a single limiter for multiple stage execution
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneTopNSourceOperator.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/LuceneTopNSourceOperator.java
@@ -12,14 +12,13 @@
 import org.apache.lucene.search.CollectionTerminatedException;
 import org.apache.lucene.search.FieldDoc;
 import org.apache.lucene.search.LeafCollector;
-import org.apache.lucene.search.Query;
 import org.apache.lucene.search.ScoreDoc;
+import org.apache.lucene.search.ScoreMode;
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.TopDocsCollector;
 import org.apache.lucene.search.TopFieldCollectorManager;
 import org.apache.lucene.search.TopScoreDocCollectorManager;
-import org.apache.lucene.search.Weight;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.compute.data.BlockFactory;
 import org.elasticsearch.compute.data.DocBlock;
@@ -44,9 +43,6 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
-import static org.apache.lucene.search.ScoreMode.TOP_DOCS;
-import static org.apache.lucene.search.ScoreMode.TOP_DOCS_WITH_SCORES;
-
 /**
  * Source operator that builds Pages out of the output of a TopFieldCollector (aka TopN)
  */
@@ -73,7 +69,7 @@ public Factory(
                 taskConcurrency,
                 limit,
                 needsScore,
-                needsScore ? TOP_DOCS_WITH_SCORES : TOP_DOCS
+                scoreModeFunction(sorts, needsScore)
             );
             this.maxPageSize = maxPageSize;
             this.sorts = sorts;
@@ -323,18 +319,11 @@ static final class ScoringPerShardCollector extends PerShardCollector {
         }
     }
 
-    private static Function<ShardContext, Weight> weightFunction(
-        Function<ShardContext, Query> queryFunction,
-        List<SortBuilder<?>> sorts,
-        boolean needsScore
-    ) {
+    private static Function<ShardContext, ScoreMode> scoreModeFunction(List<SortBuilder<?>> sorts, boolean needsScore) {
         return ctx -> {
-            final var query = queryFunction.apply(ctx);
-            final var searcher = ctx.searcher();
             try {
                 // we create a collector with a limit of 1 to determine the appropriate score mode to use.
-                var scoreMode = newPerShardCollector(ctx, sorts, needsScore, 1).collector.scoreMode();
-                return searcher.createWeight(searcher.rewrite(query), scoreMode, 1);
+                return newPerShardCollector(ctx, sorts, needsScore, 1).collector.scoreMode();
             } catch (IOException e) {
                 throw new UncheckedIOException(e);
             }
diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
@@ -63,7 +63,7 @@ private TimeSeriesSortedSourceOperatorFactory(
             taskConcurrency,
             limit,
             false,
-            ScoreMode.COMPLETE_NO_SCORES
+            shardContext -> ScoreMode.COMPLETE_NO_SCORES
         );
         this.maxPageSize = maxPageSize;
     }
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java
@@ -358,9 +358,6 @@ public static void deleteInferenceEndpoints(RestClient client) throws IOExceptio
                 }
             }
         }
-        deleteSparseEmbeddingInferenceEndpoint(client);
-        deleteRerankInferenceEndpoint(client);
-        deleteCompletionInferenceEndpoint(client);
     }
 
     /** The semantic_text mapping type require an inference endpoint that needs to be setup before creating the index. */
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec
@@ -2316,8 +2316,7 @@ warning:Line 2:9: java.lang.IllegalArgumentException: single-value function enco
 2023-10-23T13:55:01.544Z|Connected to 10.1.0.1
 ;
 
-mvStringEqualsLongString
-required_capability: mv_warn
+mvStringEqualsLongString#[skip:-8.12.99,reason:changed how we push down equals]
 
 FROM mv_text
 | WHERE message == "More than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100"
@@ -2345,8 +2344,7 @@ warning:Line 2:9: java.lang.IllegalArgumentException: single-value function enco
 2023-10-23T13:55:01.546Z|More than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100
 ;
 
-mvStringNotEqualsFound
-required_capability: mv_warn
+mvStringNotEqualsFound#[skip:-8.12.99,reason:changed how we push down not equals]
 
 FROM mv_text
 | WHERE message != "Connected to 10.1.0.1"
@@ -2359,8 +2357,7 @@ warning:Line 2:9: java.lang.IllegalArgumentException: single-value function enco
 2023-10-23T13:55:01.546Z|More than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100
 ;
 
-mvStringNotEqualsLong
-required_capability: mv_warn
+mvStringNotEqualsLong#[skip:-8.12.99,reason:changed how we push down not equals]
 
 FROM mv_text
 | WHERE message != "More than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100"
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
@@ -956,6 +956,11 @@ public enum Cap {
          */
         PARAMETER_FOR_LIMIT,
 
+        /**
+         * Changed and normalized the LIMIT error message.
+         */
+        NORMALIZED_LIMIT_ERROR_MESSAGE,
+
         /**
          * Enable support for index aliases in lookup joins
          */
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java
@@ -376,21 +376,22 @@ public PlanFactory visitWhereCommand(EsqlBaseParser.WhereCommandContext ctx) {
     public PlanFactory visitLimitCommand(EsqlBaseParser.LimitCommandContext ctx) {
         Source source = source(ctx);
         Object val = expression(ctx.constant()).fold(FoldContext.small() /* TODO remove me */);
-        if (val instanceof Integer i) {
-            if (i < 0) {
-                throw new ParsingException(source, "Invalid value for LIMIT [" + i + "], expecting a non negative integer");
-            }
+        if (val instanceof Integer i && i >= 0) {
             return input -> new Limit(source, new Literal(source, i, DataType.INTEGER), input);
-        } else {
-            throw new ParsingException(
-                source,
-                "Invalid value for LIMIT ["
-                    + BytesRefs.toString(val)
-                    + ": "
-                    + (expression(ctx.constant()).dataType() == KEYWORD ? "String" : val.getClass().getSimpleName())
-                    + "], expecting a non negative integer"
-            );
         }
+
+        String valueType = expression(ctx.constant()).dataType().typeName();
+
+        throw new ParsingException(
+            source,
+            "value of ["
+                + source.text()
+                + "] must be a non negative integer, found value ["
+                + ctx.constant().getText()
+                + "] type ["
+                + valueType
+                + "]"
+        );
     }
 
     @Override
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java
@@ -19,7 +19,10 @@
 import org.elasticsearch.xpack.esql.index.EsIndex;
 import org.elasticsearch.xpack.esql.index.IndexResolution;
 import org.elasticsearch.xpack.esql.parser.EsqlParser;
+import org.elasticsearch.xpack.esql.parser.ParserUtils;
 import org.elasticsearch.xpack.esql.parser.ParsingException;
+import org.elasticsearch.xpack.esql.parser.QueryParam;
+import org.elasticsearch.xpack.esql.parser.QueryParams;
 import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
 import org.elasticsearch.xpack.esql.plan.logical.Row;
 import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter;
@@ -153,9 +156,34 @@ public void testJoinTwiceOnTheSameField_TwoLookups() {
     }
 
     public void testInvalidLimit() {
-        assertEquals("1:13: Invalid value for LIMIT [foo: String], expecting a non negative integer", error("row a = 1 | limit \"foo\""));
-        assertEquals("1:13: Invalid value for LIMIT [1.2: Double], expecting a non negative integer", error("row a = 1 | limit 1.2"));
-        assertEquals("1:13: Invalid value for LIMIT [-1], expecting a non negative integer", error("row a = 1 | limit -1"));
+        assertLimitWithAndWithoutParams("foo", "\"foo\"", DataType.KEYWORD);
+        assertLimitWithAndWithoutParams(1.2, "1.2", DataType.DOUBLE);
+        assertLimitWithAndWithoutParams(-1, "-1", DataType.INTEGER);
+        assertLimitWithAndWithoutParams(true, "true", DataType.BOOLEAN);
+        assertLimitWithAndWithoutParams(false, "false", DataType.BOOLEAN);
+        assertLimitWithAndWithoutParams(null, "null", DataType.NULL);
+    }
+
+    private void assertLimitWithAndWithoutParams(Object value, String valueText, DataType type) {
+        assertEquals(
+            "1:13: value of [limit "
+                + valueText
+                + "] must be a non negative integer, found value ["
+                + valueText
+                + "] type ["
+                + type.typeName()
+                + "]",
+            error("row a = 1 | limit " + valueText)
+        );
+
+        assertEquals(
+            "1:13: value of [limit ?param] must be a non negative integer, found value [?param] type [" + type.typeName() + "]",
+            error(
+                "row a = 1 | limit ?param",
+                new QueryParams(List.of(new QueryParam("param", value, type, ParserUtils.ParamClassification.VALUE)))
+            )
+        );
+
     }
 
     public void testInvalidSample() {
@@ -177,13 +205,20 @@ public void testInvalidSample() {
         );
     }
 
-    private String error(String query) {
-        ParsingException e = expectThrows(ParsingException.class, () -> defaultAnalyzer.analyze(parser.createStatement(query, TEST_CFG)));
+    private String error(String query, QueryParams params) {
+        ParsingException e = expectThrows(
+            ParsingException.class,
+            () -> defaultAnalyzer.analyze(parser.createStatement(query, params, TEST_CFG))
+        );
         String message = e.getMessage();
         assertTrue(message.startsWith("line "));
         return message.substring("line ".length());
     }
 
+    private String error(String query) {
+        return error(query, new QueryParams());
+    }
+
     private static IndexResolution loadIndexResolution(String name) {
         return IndexResolution.valid(new EsIndex(INDEX_NAME, LoadMapping.loadMapping(name)));
     }
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
@@ -987,10 +987,6 @@ public void testBasicLimitCommand() {
         assertThat(limit.children().get(0).children().get(0), instanceOf(UnresolvedRelation.class));
     }
 
-    public void testLimitConstraints() {
-        expectError("from text | limit -1", "line 1:13: Invalid value for LIMIT [-1], expecting a non negative integer");
-    }
-
     public void testBasicSortCommand() {
         LogicalPlan plan = statement("from text | where true | sort a+b asc nulls first, x desc nulls last | sort y asc | sort z desc");
         assertThat(plan, instanceOf(OrderBy.class));
diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/10_basic.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/10_basic.yml
@@ -555,11 +555,11 @@ FROM EVAL SORT LIMIT with documents_found:
         - method: POST
           path: /_query
           parameters: [ ]
-          capabilities: [ parameter_for_limit ]
+          capabilities: [ normalized_limit_error_message ]
       reason: "named or positional parameters for field names"
 
   - do:
-      catch: "/Invalid value for LIMIT \\[foo: String\\], expecting a non negative integer/"
+      catch: "/value of \\[limit \\?l\\] must be a non negative integer, found value \\[\\?l\\] type \\[keyword\\]/"
       esql.query:
         body:
           query: 'from test | limit ?l'

Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ public Factory(`
`56`	`56`	`taskConcurrency,`
`57`	`57`	`limit,`
`58`	`58`	`false,`
`59`		`- ScoreMode.COMPLETE_NO_SCORES`
	`59`	`+ shardContext -> ScoreMode.COMPLETE_NO_SCORES`
`60`	`60`	`);`
`61`	`61`	`}`
`62`	`62`