From 14c65519780012ca463c637a601ad862f9b8722b Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Fri, 3 Oct 2025 18:21:44 -0700 Subject: [PATCH 1/5] Avoid rewrite round_to with expensive queries --- .../local/ReplaceRoundToWithQueryAndTags.java | 63 ++++++++++++++++++- .../ReplaceRoundToWithQueryAndTagsTests.java | 61 ++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java index 4cdef35437ebf..09f694029514f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java @@ -7,7 +7,16 @@ package org.elasticsearch.xpack.esql.optimizer.rules.physical.local; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.query.BoolQueryBuilder; +import org.elasticsearch.index.query.MatchAllQueryBuilder; +import org.elasticsearch.index.query.MatchNoneQueryBuilder; +import org.elasticsearch.index.query.MultiTermQueryBuilder; +import org.elasticsearch.index.query.PrefixQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.RegexpQueryBuilder; +import org.elasticsearch.index.query.TermsQueryBuilder; +import org.elasticsearch.index.query.WildcardQueryBuilder; import org.elasticsearch.logging.LogManager; import org.elasticsearch.logging.Logger; import org.elasticsearch.xpack.esql.core.expression.Alias; @@ -275,7 +284,11 @@ protected PhysicalPlan rule(EvalExec evalExec, LocalPhysicalOptimizerContext ctx if (roundTos.size() == 1) { RoundTo roundTo = roundTos.get(0); int count = roundTo.points().size(); - int roundingPointsUpperLimit = roundingPointsThreshold(ctx); + int roundingPointsUpperLimit = adjustedRoundingPointsThreshold( + roundingPointsThreshold(ctx), + queryExec.query(), + queryExec.indexMode() + ); if (count > roundingPointsUpperLimit) { logger.debug( "Skipping RoundTo push down for [{}], as it has [{}] points, which is more than [{}]", @@ -485,4 +498,52 @@ private int roundingPointsThreshold(LocalPhysicalOptimizerContext ctx) { } return roundingPointsThreshold; } + + /** + * If the main query is expensive (such as including wildcard queries), executing more queries with tags is slower and more costly + * than executing fewer queries without tags and then reading points and rounding. The rounding points threshold is treated as the + * maximum number of clauses allowed to execute. We estimate the number of clauses in the main query and adjust the threshold so + * that the total number of clauses does not exceed the limit by too much. Some expensive queries count as more than one clause; + * for example, a wildcard query counts as 5 clauses, and a terms query counts as the number of terms. + */ + static int adjustedRoundingPointsThreshold(int threshold, QueryBuilder query, IndexMode indexMode) { + int clauses = estimateQueryClauses(query) + 1; + if (indexMode == IndexMode.TIME_SERIES) { + // No doc partitioning for time_series sources; increase the threshold to trade overhead for parallelism. + threshold *= 2; + } + return Math.ceilDiv(threshold, clauses); + } + + static int estimateQueryClauses(QueryBuilder q) { + if (q == null || q instanceof MatchAllQueryBuilder || q instanceof MatchNoneQueryBuilder) { + return 0; + } + if (q instanceof WildcardQueryBuilder || q instanceof RegexpQueryBuilder || q instanceof PrefixQueryBuilder) { + return 5; + } + if (q instanceof MultiTermQueryBuilder) { + return 3; + } + if (q instanceof TermsQueryBuilder terms && terms.values() != null) { + return terms.values().size(); + } + if (q instanceof BoolQueryBuilder bq) { + int total = 0; + for (var c : bq.filter()) { + total += estimateQueryClauses(c); + } + for (var c : bq.must()) { + total += estimateQueryClauses(c); + } + for (var c : bq.should()) { + total += estimateQueryClauses(c); + } + for (var c : bq.mustNot()) { + total += Math.max(2, estimateQueryClauses(c)); + } + return total; + } + return 1; + } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java index 14ddb647946bb..8fe24471de372 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java @@ -53,6 +53,7 @@ import java.util.Locale; import java.util.Map; import java.util.stream.Collectors; +import java.util.stream.IntStream; import static org.elasticsearch.compute.aggregation.AggregatorMode.FINAL; import static org.elasticsearch.compute.aggregation.AggregatorMode.SINGLE; @@ -67,6 +68,8 @@ import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.DEFAULT_DATE_TIME_FORMATTER; import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateNanosToLong; import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToLong; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; //@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE", reason = "debug") @@ -537,6 +540,64 @@ public void testRoundToTransformToQueryAndTagsWithCustomizedUpperLimit() { } } + static String points(int numPoints) { + return IntStream.range(0, numPoints).mapToObj(Integer::toString).collect(Collectors.joining(",")); + } + + public void testAdjustThresholdForQueries() { + { + String q = String.format(Locale.ROOT, """ + from test + | stats count(*) by x = round_to(integer, %s) + """, points(between(1, 128))); + PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); + assertThat(esQuery.queryBuilderAndTags().size(), greaterThan(1)); + } + { + String q = String.format(Locale.ROOT, """ + from test + | where date >= "2023-10-19" + | stats count(*) by x = round_to(integer, %s) + """, points(between(1, 63))); + PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); + assertThat(esQuery.queryBuilderAndTags().size(), greaterThan(1)); + } + { + String q = String.format(Locale.ROOT, """ + from test + | where date >= "2023-10-19" + | stats count(*) by x = round_to(integer, %s) + """, points(between(65, 128))); + PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); + assertThat(esQuery.queryBuilderAndTags().size(), equalTo(1)); + } + { + String q = String.format(Locale.ROOT, """ + from test + | where date >= "2023-10-19" + | where keyword LIKE "w*" + | stats count(*) by x = round_to(integer, %s) + """, points(between(1, 19))); + PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); + assertThat(esQuery.queryBuilderAndTags().size(), greaterThan(1)); + } + { + String q = String.format(Locale.ROOT, """ + from test + | where date >= "2023-10-19" + | where keyword LIKE "*w*" + | stats count(*) by x = round_to(integer, %s) + """, points(between(20, 128))); + PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); + assertThat(esQuery.queryBuilderAndTags().size(), equalTo(1)); + } + } + private static void verifyQueryAndTags(List expected, List actual) { assertEquals(expected.size(), actual.size()); for (int i = 0; i < expected.size(); i++) { From c9ce8b81890d6aa0f3ea17c885e017ea31e8795d Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Fri, 3 Oct 2025 21:08:55 -0700 Subject: [PATCH 2/5] Update docs/changelog/135987.yaml --- docs/changelog/135987.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/135987.yaml diff --git a/docs/changelog/135987.yaml b/docs/changelog/135987.yaml new file mode 100644 index 0000000000000..6ff44934c7046 --- /dev/null +++ b/docs/changelog/135987.yaml @@ -0,0 +1,5 @@ +pr: 135987 +summary: Avoid rewrite `round_to` with expensive queries +area: ES|QL +type: bug +issues: [] From b055ae08a47e58afe39ed92a8337397cc4112a4e Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 4 Oct 2025 00:44:10 -0700 Subject: [PATCH 3/5] Add fuzzy --- .../physical/local/ReplaceRoundToWithQueryAndTags.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java index 09f694029514f..1b4fa2dcb68cb 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java @@ -9,6 +9,7 @@ import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.query.BoolQueryBuilder; +import org.elasticsearch.index.query.FuzzyQueryBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.MatchNoneQueryBuilder; import org.elasticsearch.index.query.MultiTermQueryBuilder; @@ -519,7 +520,10 @@ static int estimateQueryClauses(QueryBuilder q) { if (q == null || q instanceof MatchAllQueryBuilder || q instanceof MatchNoneQueryBuilder) { return 0; } - if (q instanceof WildcardQueryBuilder || q instanceof RegexpQueryBuilder || q instanceof PrefixQueryBuilder) { + if (q instanceof WildcardQueryBuilder + || q instanceof RegexpQueryBuilder + || q instanceof PrefixQueryBuilder + || q instanceof FuzzyQueryBuilder) { return 5; } if (q instanceof MultiTermQueryBuilder) { From 2f7fd824ae8164ce7817de0217fc57c8cc87f098 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 4 Oct 2025 01:08:59 -0700 Subject: [PATCH 4/5] Add range with points --- .../local/ReplaceRoundToWithQueryAndTags.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java index 1b4fa2dcb68cb..4b21cddee6a6b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTags.java @@ -15,6 +15,7 @@ import org.elasticsearch.index.query.MultiTermQueryBuilder; import org.elasticsearch.index.query.PrefixQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.RangeQueryBuilder; import org.elasticsearch.index.query.RegexpQueryBuilder; import org.elasticsearch.index.query.TermsQueryBuilder; import org.elasticsearch.index.query.WildcardQueryBuilder; @@ -40,6 +41,8 @@ import org.elasticsearch.xpack.esql.plan.physical.EsQueryExec; import org.elasticsearch.xpack.esql.plan.physical.EvalExec; import org.elasticsearch.xpack.esql.plan.physical.PhysicalPlan; +import org.elasticsearch.xpack.esql.querydsl.query.SingleValueQuery; +import org.elasticsearch.xpack.esql.stats.SearchStats; import java.time.ZoneId; import java.util.ArrayList; @@ -286,6 +289,7 @@ protected PhysicalPlan rule(EvalExec evalExec, LocalPhysicalOptimizerContext ctx RoundTo roundTo = roundTos.get(0); int count = roundTo.points().size(); int roundingPointsUpperLimit = adjustedRoundingPointsThreshold( + ctx.searchStats(), roundingPointsThreshold(ctx), queryExec.query(), queryExec.indexMode() @@ -507,8 +511,8 @@ private int roundingPointsThreshold(LocalPhysicalOptimizerContext ctx) { * that the total number of clauses does not exceed the limit by too much. Some expensive queries count as more than one clause; * for example, a wildcard query counts as 5 clauses, and a terms query counts as the number of terms. */ - static int adjustedRoundingPointsThreshold(int threshold, QueryBuilder query, IndexMode indexMode) { - int clauses = estimateQueryClauses(query) + 1; + static int adjustedRoundingPointsThreshold(SearchStats stats, int threshold, QueryBuilder query, IndexMode indexMode) { + int clauses = estimateQueryClauses(stats, query) + 1; if (indexMode == IndexMode.TIME_SERIES) { // No doc partitioning for time_series sources; increase the threshold to trade overhead for parallelism. threshold *= 2; @@ -516,7 +520,7 @@ static int adjustedRoundingPointsThreshold(int threshold, QueryBuilder query, In return Math.ceilDiv(threshold, clauses); } - static int estimateQueryClauses(QueryBuilder q) { + static int estimateQueryClauses(SearchStats stats, QueryBuilder q) { if (q == null || q instanceof MatchAllQueryBuilder || q instanceof MatchNoneQueryBuilder) { return 0; } @@ -526,25 +530,33 @@ static int estimateQueryClauses(QueryBuilder q) { || q instanceof FuzzyQueryBuilder) { return 5; } + if (q instanceof RangeQueryBuilder r) { + // with points count 1, without count 3 + return stats.min(new FieldAttribute.FieldName(r.fieldName())) != null ? 1 : 3; + } if (q instanceof MultiTermQueryBuilder) { return 3; } if (q instanceof TermsQueryBuilder terms && terms.values() != null) { return terms.values().size(); } + if (q instanceof SingleValueQuery.Builder b) { + // ignore the single_value clause + return Math.max(1, estimateQueryClauses(stats, b.next())); + } if (q instanceof BoolQueryBuilder bq) { int total = 0; for (var c : bq.filter()) { - total += estimateQueryClauses(c); + total += estimateQueryClauses(stats, c); } for (var c : bq.must()) { - total += estimateQueryClauses(c); + total += estimateQueryClauses(stats, c); } for (var c : bq.should()) { - total += estimateQueryClauses(c); + total += estimateQueryClauses(stats, c); } for (var c : bq.mustNot()) { - total += Math.max(2, estimateQueryClauses(c)); + total += Math.max(2, estimateQueryClauses(stats, c)); } return total; } From e0ec4206667e6c1e28391a991f400c3d3343703e Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sat, 4 Oct 2025 01:09:16 -0700 Subject: [PATCH 5/5] harden tests --- .../ReplaceRoundToWithQueryAndTagsTests.java | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java index 8fe24471de372..a97d15b40be3c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/ReplaceRoundToWithQueryAndTagsTests.java @@ -69,7 +69,6 @@ import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateNanosToLong; import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToLong; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; //@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE", reason = "debug") @@ -540,61 +539,71 @@ public void testRoundToTransformToQueryAndTagsWithCustomizedUpperLimit() { } } - static String points(int numPoints) { + static String pointArray(int numPoints) { return IntStream.range(0, numPoints).mapToObj(Integer::toString).collect(Collectors.joining(",")); } + static int queryAndTags(PhysicalPlan plan) { + EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); + return esQuery.queryBuilderAndTags().size(); + } + public void testAdjustThresholdForQueries() { { + int points = between(2, 127); String q = String.format(Locale.ROOT, """ from test | stats count(*) by x = round_to(integer, %s) - """, points(between(1, 128))); + """, pointArray(points)); PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); - EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); - assertThat(esQuery.queryBuilderAndTags().size(), greaterThan(1)); + int queryAndTags = queryAndTags(plan); + assertThat(queryAndTags, equalTo(points + 1)); // include null bucket } { + int points = between(2, 64); String q = String.format(Locale.ROOT, """ from test | where date >= "2023-10-19" | stats count(*) by x = round_to(integer, %s) - """, points(between(1, 63))); - PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); - EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); - assertThat(esQuery.queryBuilderAndTags().size(), greaterThan(1)); + """, pointArray(points)); + var plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + int queryAndTags = queryAndTags(plan); + assertThat(queryAndTags, equalTo(points + 1)); // include null bucket } { + int points = between(65, 128); String q = String.format(Locale.ROOT, """ from test | where date >= "2023-10-19" | stats count(*) by x = round_to(integer, %s) - """, points(between(65, 128))); - PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); - EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); - assertThat(esQuery.queryBuilderAndTags().size(), equalTo(1)); + """, pointArray(points)); + var plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + int queryAndTags = queryAndTags(plan); + assertThat(queryAndTags, equalTo(1)); // no rewrite } { + int points = between(2, 19); String q = String.format(Locale.ROOT, """ from test | where date >= "2023-10-19" | where keyword LIKE "w*" | stats count(*) by x = round_to(integer, %s) - """, points(between(1, 19))); - PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); - EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); - assertThat(esQuery.queryBuilderAndTags().size(), greaterThan(1)); + """, pointArray(points)); + var plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); + int queryAndTags = queryAndTags(plan); + assertThat("points=" + points, queryAndTags, equalTo(points + 1)); // include null bucket } { + int points = between(20, 128); String q = String.format(Locale.ROOT, """ from test | where date >= "2023-10-19" | where keyword LIKE "*w*" | stats count(*) by x = round_to(integer, %s) - """, points(between(20, 128))); + """, pointArray(points)); PhysicalPlan plan = plannerOptimizer.plan(q, searchStats, makeAnalyzer("mapping-all-types.json")); - EsQueryExec esQuery = (EsQueryExec) plan.collectFirstChildren(EsQueryExec.class::isInstance).getFirst(); - assertThat(esQuery.queryBuilderAndTags().size(), equalTo(1)); + int queryAndTags = queryAndTags(plan); + assertThat("points=" + points, queryAndTags, equalTo(1)); // no rewrite } }