Skip to content

Commit ca6aef7

Browse files
committed
Revert "[performance](agg) support count push agg in no null column"
This reverts commit 13cce98.
1 parent b2307b0 commit ca6aef7

File tree

4 files changed

+30
-99
lines changed

4 files changed

+30
-99
lines changed

be/src/olap/rowset/segment_v2/segment_iterator.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,11 @@ void SegmentIterator::_init_row_bitmap_by_condition_cache() {
131131
if (_find_condition_cache) {
132132
DorisMetrics::instance()->condition_cache_hit_count->increment(1);
133133
if (_opts.runtime_state) {
134-
LOG(INFO) << "Condition cache hit, query id: " << print_id(_opts.runtime_state->query_id()) <<
135-
", segment id: " << _segment->id() <<
136-
", cache digest: " << _opts.condition_cache_digest <<
137-
", rowset id: " << _opts.rowset_id.to_string();
134+
LOG(INFO) << "Condition cache hit, query id: "
135+
<< print_id(_opts.runtime_state->query_id())
136+
<< ", segment id: " << _segment->id()
137+
<< ", cache digest: " << _opts.condition_cache_digest
138+
<< ", rowset id: " << _opts.rowset_id.to_string();
138139
}
139140
}
140141

@@ -2422,10 +2423,11 @@ Status SegmentIterator::next_batch(vectorized::Block* block) {
24222423
auto* condition_cache = ConditionCache::instance();
24232424
ConditionCache::CacheKey cache_key(_opts.rowset_id, _segment->id(),
24242425
_opts.condition_cache_digest);
2425-
LOG(INFO) << "Condition cache insert, query id: " << print_id(_opts.runtime_state->query_id()) <<
2426-
", rowset id: " << _opts.rowset_id.to_string() <<
2427-
", segment id: " << _segment->id() <<
2428-
", cache digest: " << _opts.condition_cache_digest;
2426+
LOG(INFO) << "Condition cache insert, query id: "
2427+
<< print_id(_opts.runtime_state->query_id())
2428+
<< ", rowset id: " << _opts.rowset_id.to_string()
2429+
<< ", segment id: " << _segment->id()
2430+
<< ", cache digest: " << _opts.condition_cache_digest;
24292431
condition_cache->insert(cache_key, std::move(_condition_cache));
24302432
}
24312433
return res;

be/src/vec/exprs/vruntimefilter_wrapper.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,13 @@ class VRuntimeFilterWrapper final : public VExpr {
6262
const std::string& expr_name() const override;
6363
const VExprSPtrs& children() const override { return _impl->children(); }
6464

65-
uint64_t get_digest(uint64_t seed) const override { return _impl->get_digest(seed); }
65+
uint64_t get_digest(uint64_t seed) const override {
66+
seed = _impl->get_digest(seed);
67+
if (seed) {
68+
return HashUtil::crc_hash64(&_null_aware, sizeof(_null_aware), seed);
69+
}
70+
return seed;
71+
}
6672

6773
VExprSPtr get_impl() const override { return _impl; }
6874

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java

Lines changed: 13 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161

6262
import com.google.common.collect.ImmutableList;
6363

64-
import java.util.HashSet;
6564
import java.util.List;
6665
import java.util.Map;
6766
import java.util.Optional;
@@ -559,54 +558,28 @@ private LogicalAggregate<? extends Plan> storageLayerAggregate(
559558
}
560559

561560
Set<AggregateFunction> aggregateFunctions = aggregate.getAggregateFunctions();
562-
// Use for loop to replace Stream API
563-
Set<Class<? extends AggregateFunction>> functionClasses = new HashSet<>();
564-
Map<Class<? extends AggregateFunction>, PushDownAggOp> supportedAgg = PushDownAggOp.supportedFunctions();
565-
566-
boolean containsCount = false;
567-
Set<SlotReference> checkNullSlots = new HashSet<>();
568-
569-
// Single loop through aggregateFunctions to handle multiple logic
570-
for (AggregateFunction function : aggregateFunctions) {
571-
Class<? extends AggregateFunction> functionClass = function.getClass();
572-
functionClasses.add(functionClass);
573-
// Check if any function has arity > 1
574-
if (function.arity() > 1) {
575-
return canNotPush;
576-
}
577-
578-
// Check if contains Count function
579-
if (functionClass.equals(Count.class)) {
580-
containsCount = true;
581-
if (!function.getArguments().isEmpty()) {
582-
Expression arg0 = function.getArguments().get(0);
583-
if (arg0 instanceof SlotReference) {
584-
checkNullSlots.add((SlotReference) arg0);
585-
} else if (arg0 instanceof Cast) {
586-
Expression child0 = arg0.child(0);
587-
if (child0 instanceof SlotReference) {
588-
checkNullSlots.add((SlotReference) child0);
589-
}
590-
}
591-
}
592-
}
561+
Set<Class<? extends AggregateFunction>> functionClasses = aggregateFunctions
562+
.stream()
563+
.map(AggregateFunction::getClass)
564+
.collect(Collectors.toSet());
593565

594-
// Check if function is supported by supportedAgg
595-
if (!supportedAgg.containsKey(functionClass)) {
596-
return canNotPush;
597-
}
566+
Map<Class<? extends AggregateFunction>, PushDownAggOp> supportedAgg = PushDownAggOp.supportedFunctions();
567+
if (!supportedAgg.keySet().containsAll(functionClasses)) {
568+
return canNotPush;
598569
}
599-
600570
if (logicalScan instanceof LogicalOlapScan) {
601571
LogicalOlapScan logicalOlapScan = (LogicalOlapScan) logicalScan;
602572
KeysType keysType = logicalOlapScan.getTable().getKeysType();
603-
if (containsCount && keysType != KeysType.DUP_KEYS) {
573+
if (functionClasses.contains(Count.class) && keysType != KeysType.DUP_KEYS) {
604574
return canNotPush;
605575
}
606-
if (containsCount && logicalOlapScan.isDirectMvScan()) {
576+
if (functionClasses.contains(Count.class) && logicalOlapScan.isDirectMvScan()) {
607577
return canNotPush;
608578
}
609579
}
580+
if (aggregateFunctions.stream().anyMatch(fun -> fun.arity() > 1)) {
581+
return canNotPush;
582+
}
610583

611584
// TODO: refactor this to process slot reference or expression together
612585
boolean onlyContainsSlotOrNumericCastSlot = aggregateFunctions.stream()
@@ -692,7 +665,7 @@ private LogicalAggregate<? extends Plan> storageLayerAggregate(
692665
// NULL value behavior in `count` function is zero, so
693666
// we should not use row_count to speed up query. the col
694667
// must be not null
695-
if (column.isAllowNull() && checkNullSlots.contains(slot)) {
668+
if (column.isAllowNull()) {
696669
return canNotPush;
697670
}
698671
}

regression-test/suites/nereids_p0/explain/test_pushdown_explain.groovy

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -74,56 +74,6 @@ suite("test_pushdown_explain") {
7474
contains "pushAggOp=NONE"
7575
}
7676

77-
// Test cases for NULL column handling in count pushdown optimization
78-
sql "DROP TABLE IF EXISTS test_null_columns"
79-
sql """ CREATE TABLE `test_null_columns` (
80-
`id` INT NOT NULL COMMENT 'ID',
81-
`nullable_col` VARCHAR(11) NULL COMMENT 'Nullable column',
82-
`non_nullable_col` VARCHAR(11) NOT NULL COMMENT 'Non-nullable column'
83-
) ENGINE=OLAP
84-
DUPLICATE KEY(`id`)
85-
DISTRIBUTED BY HASH(`id`) BUCKETS 48
86-
PROPERTIES (
87-
"replication_allocation" = "tag.location.default: 1",
88-
"min_load_replica_num" = "-1",
89-
"is_being_synced" = "false",
90-
"colocate_with" = "groupa1",
91-
"storage_format" = "V2",
92-
"light_schema_change" = "true",
93-
"disable_auto_compaction" = "false",
94-
"enable_single_replica_compaction" = "false"
95-
); """
96-
sql """ insert into test_null_columns values(1, NULL, "value1"); """
97-
sql """ insert into test_null_columns values(2, NULL, "value2"); """
98-
sql """ insert into test_null_columns values(3, "not_null", "value3"); """
99-
100-
// Test count(1) and count(*) with NULL columns - should push Count optimization
101-
explain {
102-
sql("select count(1) from test_null_columns;")
103-
contains "pushAggOp=COUNT"
104-
}
105-
explain {
106-
sql("select count(*) from test_null_columns;")
107-
contains "pushAggOp=COUNT"
108-
}
109-
110-
explain {
111-
sql("select count(non_nullable_col), min(non_nullable_col), max(non_nullable_col) from test_null_columns;")
112-
contains "pushAggOp=MIX"
113-
}
114-
explain {
115-
sql("select count(), min(non_nullable_col), max(non_nullable_col) from test_null_columns;")
116-
contains "pushAggOp=MIX"
117-
}
118-
explain {
119-
sql("select count(*), min(non_nullable_col), max(non_nullable_col) from test_null_columns;")
120-
contains "pushAggOp=MIX"
121-
}
122-
explain {
123-
sql("select count(nullable_col), min(nullable_col), max(nullable_col) from test_null_columns;")
124-
contains "pushAggOp=NONE"
125-
}
126-
12777
sql "DROP TABLE IF EXISTS table_unique0"
12878
sql """
12979
CREATE TABLE `table_unique0` (

0 commit comments

Comments
 (0)