Skip to content

Commit 8103eb4

Browse files
authored
ESQL: fix COUNT filter pushdown (#117503) (#117652)
* ESQL: fix COUNT filter pushdown (#117503) If `COUNT` agg has a filter applied, this must also be push down to source. This currently does not happen, but this issue is masked currently by two factors: * a logical optimisation, `ExtractAggregateCommonFilter` that extracts the filter out of the STATS entirely (and pushes it to source then from a `WHERE`); * the phisical plan optimisation implementing the push down, `PushStatsToSource`, currently only applies if there's just one agg function to push down. However, this fix needs to be applied since: * it's still present in versions prior to `ExtractAggregateCommonFilter` introduction; * the defect might resurface when the restriction in `PushStatsToSource` is lifted. Fixes #115522. (cherry picked from commit 560e0c5) * 8.16 adaptation
1 parent f0fe030 commit 8103eb4

File tree

4 files changed

+117
-0
lines changed

4 files changed

+117
-0
lines changed

docs/changelog/117503.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 117503
2+
summary: Fix COUNT filter pushdown
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 115522

x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,6 +2537,57 @@ c2:l |c2_f:l |m2:i |m2_f:i |c:l
25372537
1 |1 |5 |5 |21
25382538
;
25392539

2540+
simpleCountOnFieldWithFilteringAndNoGrouping
2541+
required_capability: per_agg_filtering
2542+
from employees
2543+
| stats c1 = count(emp_no) where emp_no < 10042
2544+
;
2545+
2546+
c1:long
2547+
41
2548+
;
2549+
2550+
simpleCountOnFieldWithFilteringOnDifferentFieldAndNoGrouping
2551+
required_capability: per_agg_filtering
2552+
from employees
2553+
| stats c1 = count(hire_date) where emp_no < 10042
2554+
;
2555+
2556+
c1:long
2557+
41
2558+
;
2559+
2560+
simpleCountOnStarWithFilteringAndNoGrouping
2561+
required_capability: per_agg_filtering
2562+
from employees
2563+
| stats c1 = count(*) where emp_no < 10042
2564+
;
2565+
2566+
c1:long
2567+
41
2568+
;
2569+
2570+
simpleCountWithFilteringAndNoGroupingOnFieldWithNulls
2571+
required_capability: per_agg_filtering
2572+
from employees
2573+
| stats c1 = count(birth_date) where emp_no <= 10050
2574+
;
2575+
2576+
c1:long
2577+
40
2578+
;
2579+
2580+
2581+
simpleCountWithFilteringAndNoGroupingOnFieldWithMultivalues
2582+
required_capability: per_agg_filtering
2583+
from employees
2584+
| stats c1 = count(job_positions) where emp_no <= 10003
2585+
;
2586+
2587+
c1:long
2588+
3
2589+
;
2590+
25402591
filterIsAlwaysTrue
25412592
required_capability: per_agg_filtering
25422593
FROM employees

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.xpack.esql.core.expression.Expression;
1717
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
1818
import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
19+
import org.elasticsearch.xpack.esql.core.util.Queries;
1920
import org.elasticsearch.xpack.esql.core.util.StringUtils;
2021
import org.elasticsearch.xpack.esql.expression.function.aggregate.Count;
2122
import org.elasticsearch.xpack.esql.optimizer.LocalPhysicalOptimizerContext;
@@ -25,12 +26,15 @@
2526
import org.elasticsearch.xpack.esql.plan.physical.EsStatsQueryExec;
2627
import org.elasticsearch.xpack.esql.plan.physical.PhysicalPlan;
2728
import org.elasticsearch.xpack.esql.planner.AbstractPhysicalOperationProviders;
29+
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
2830

2931
import java.util.ArrayList;
3032
import java.util.List;
3133

34+
import static java.util.Arrays.asList;
3235
import static java.util.Collections.emptyList;
3336
import static java.util.Collections.singletonList;
37+
import static org.elasticsearch.xpack.esql.optimizer.rules.physical.local.PushFiltersToSource.canPushToSource;
3438
import static org.elasticsearch.xpack.esql.plan.physical.EsStatsQueryExec.StatsType.COUNT;
3539

3640
/**
@@ -98,6 +102,13 @@ private Tuple<List<Attribute>, List<EsStatsQueryExec.Stat>> pushableStats(
98102
}
99103
}
100104
if (fieldName != null) {
105+
if (count.hasFilter()) {
106+
if (canPushToSource(count.filter(), fa -> false) == false) {
107+
return null; // can't push down
108+
}
109+
var countFilter = PlannerUtils.TRANSLATOR_HANDLER.asQuery(count.filter());
110+
query = Queries.combine(Queries.Clause.MUST, asList(countFilter.asBuilder(), query));
111+
}
101112
return new EsStatsQueryExec.Stat(fieldName, COUNT, query);
102113
}
103114
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import java.util.List;
6868
import java.util.Locale;
6969
import java.util.Map;
70+
import java.util.function.Function;
7071

7172
import static java.util.Arrays.asList;
7273
import static org.elasticsearch.compute.aggregation.AggregatorMode.FINAL;
@@ -373,6 +374,54 @@ public void testMultiCountAllWithFilter() {
373374
assertThat(plan.anyMatch(EsQueryExec.class::isInstance), is(true));
374375
}
375376

377+
@SuppressWarnings("unchecked")
378+
public void testSingleCountWithStatsFilter() {
379+
var analyzer = makeAnalyzer("mapping-default.json", new EnrichResolution());
380+
var plannerOptimizer = new TestPlannerOptimizer(config, analyzer);
381+
var plan = plannerOptimizer.plan("""
382+
from test
383+
| stats c = count(hire_date) where emp_no < 10042
384+
""", IS_SV_STATS);
385+
386+
var limit = as(plan, LimitExec.class);
387+
var agg = as(limit.child(), AggregateExec.class);
388+
assertThat(agg.getMode(), is(FINAL));
389+
var exchange = as(agg.child(), ExchangeExec.class);
390+
var esStatsQuery = as(exchange.child(), EsStatsQueryExec.class);
391+
assertThat(esStatsQuery.stats().size(), is(1));
392+
393+
Function<String, String> compact = s -> s.replaceAll("\\s+", "");
394+
assertThat(compact.apply(esStatsQuery.stats().get(0).query().toString()), is(compact.apply("""
395+
{
396+
"bool": {
397+
"must": [
398+
{
399+
"esql_single_value": {
400+
"field": "emp_no",
401+
"next": {
402+
"range": {
403+
"emp_no": {
404+
"lt": 10042,
405+
"boost": 1.0
406+
}
407+
}
408+
},
409+
"source": "emp_no < 10042@2:36"
410+
}
411+
},
412+
{
413+
"exists": {
414+
"field": "hire_date",
415+
"boost": 1.0
416+
}
417+
}
418+
],
419+
"boost": 1.0
420+
}
421+
}
422+
""")));
423+
}
424+
376425
/**
377426
* Expecting
378427
* LimitExec[1000[INTEGER]]

0 commit comments

Comments
 (0)