Skip to content

Commit e8251f6

Browse files
committed
Add more tests
Ensure that the code path with pushdown is actually covered.
1 parent becc26c commit e8251f6

File tree

2 files changed

+49
-2
lines changed

2 files changed

+49
-2
lines changed

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,9 @@ count:long | @timestamp:date
393393
0 | 2023-10-23T13:50:00.000Z
394394
;
395395

396-
multiIndexIpStatsPushableCount
396+
multiIndexIpStatsNonPushableCount
397+
// Could be pushed to Lucene if we knew whether ip fields are single valued or not.
398+
// May still be pushed down on multi-node environments if a node has only the index where client_ip is keyword.
397399
required_capability: union_types
398400
required_capability: fix_count_pushdown_for_union_types
399401

@@ -405,7 +407,9 @@ count:long
405407
14
406408
;
407409

408-
multiIndexIpStatsPushableCountEval
410+
multiIndexIpStatsNonPushableCountEval
411+
// Could be pushed to Lucene if we knew whether ip fields are single valued or not.
412+
// May still be pushed down on multi-node environments if a node has only the index where client_ip is keyword.
409413
required_capability: union_types
410414
required_capability: fix_count_pushdown_for_union_types
411415

@@ -418,6 +422,45 @@ count:long
418422
14
419423
;
420424

425+
multiIndexIpStatsNonPushableCountWithFilter
426+
// Currently not pushable: has 2 aggs and we don't consider multi-typed fields aggregatable.
427+
required_capability: union_types
428+
required_capability: fix_count_pushdown_for_union_types
429+
430+
FROM sample_data, sample_data_ts_long
431+
| STATS count_matching=count(@timestamp::long) WHERE @timestamp::long >= 1698069301543,
432+
total_count=count(@timestamp::long)
433+
;
434+
435+
count_matching:long | total_count:long
436+
2 | 14
437+
;
438+
439+
multiIndexIpStatsPushableCount
440+
required_capability: union_types
441+
required_capability: fix_count_pushdown_for_union_types
442+
443+
FROM sample_data, sample_data_ts_long
444+
| STATS count=count(@timestamp::long)
445+
;
446+
447+
count:long
448+
14
449+
;
450+
451+
multiIndexIpStatsPushableCountEval
452+
required_capability: union_types
453+
required_capability: fix_count_pushdown_for_union_types
454+
455+
FROM sample_data, sample_data_ts_long
456+
| EVAL @timestamp = @timestamp::long
457+
| STATS count=count(@timestamp)
458+
;
459+
460+
count:long
461+
14
462+
;
463+
421464
multiIndexIpStringStatsInline2
422465
required_capability: union_types
423466
required_capability: union_types_agg_cast

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ private Tuple<List<Attribute>, List<EsStatsQueryExec.Stat>> pushableStats(
103103
}
104104
if (fieldName != null) {
105105
if (count.hasFilter()) {
106+
// Note: currently, it seems like we never actually perform stats pushdown if we reach here.
107+
// That's because stats pushdown only works for 1 agg function (without BY); but in that case, filters
108+
// are extracted into a separate filter node upstream from the aggregation (and hopefully pushed into
109+
// the EsQueryExec separately).
106110
if (canPushToSource(count.filter()) == false) {
107111
return null; // can't push down
108112
}

0 commit comments

Comments
 (0)