You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "Copilot (via GitHub)" <gi...@apache.org> on 2026/04/02 02:13:08 UTC
Re: [PR] [Bug](scan) Preserve IN_LIST runtime filter predicates when key range is a scope range [doris]
Copilot commented on code in PR #62027:
URL: https://github.com/apache/doris/pull/62027#discussion_r3025518449
##########
regression-test/suites/correctness_p0/test_rf_in_list_not_erased_by_scope_range.groovy:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// This test verifies that when both MINMAX and IN runtime filters target the same
+// key column, and the IN filter's value count exceeds max_pushdown_conditions_per_column,
+// the IN_LIST predicate is NOT incorrectly erased by the key range construction logic.
+// Regression test for the bug where _build_key_ranges_and_filters() erased IN_LIST
+// predicates when the ColumnValueRange was a scope range (from MINMAX filter).
+suite("test_rf_in_list_not_erased_by_scope_range") {
+ sql "drop table if exists rf_scope_probe;"
+ sql "drop table if exists rf_scope_build;"
+
+ sql """
+ CREATE TABLE rf_scope_probe (
+ k1 BIGINT,
+ v1 INT
+ )
+ DUPLICATE KEY(k1)
+ DISTRIBUTED BY HASH(k1) BUCKETS 1
+ PROPERTIES ("replication_num" = "1");
+ """
+
+ sql """
+ CREATE TABLE rf_scope_build (
+ k1 BIGINT,
+ v1 INT
+ )
+ DUPLICATE KEY(k1)
+ DISTRIBUTED BY HASH(k1) BUCKETS 1
+ PROPERTIES ("replication_num" = "1");
+ """
+
+ // Probe table: insert 20 rows with k1 from 1 to 20.
+ // The build side will only match a subset (k1 in {2,4,6,8,10,12}).
+ // Rows NOT in this subset (k1=1,3,5,7,9,11,13..20) should be filtered out
+ // by the IN_LIST runtime filter.
+ sql """
+ INSERT INTO rf_scope_probe VALUES
+ (1, 1), (2, 2), (3, 3), (4, 4), (5, 5),
+ (6, 6), (7, 7), (8, 8), (9, 9), (10, 10),
+ (11, 11), (12, 12), (13, 13), (14, 14), (15, 15),
+ (16, 16), (17, 17), (18, 18), (19, 19), (20, 20);
+ """
+
+ // Build table: 6 distinct k1 values. This exceeds max_pushdown_conditions_per_column=5
+ // so the IN values are NOT added to ColumnValueRange, but the IN_LIST predicate is created.
+ // MINMAX range: [2, 12]
+ sql """
+ INSERT INTO rf_scope_build VALUES
+ (2, 100), (4, 200), (6, 300), (8, 400), (10, 500), (12, 600);
+ """
+
+ sql "sync;"
+
+ // Set max_pushdown_conditions_per_column to 5, so the 6 IN values exceed it.
+ // This causes IN values to NOT be added to the ColumnValueRange (it stays as
+ // a scope range from the MINMAX filter), but the IN_LIST ColumnPredicate is still created.
+ sql "set max_pushdown_conditions_per_column = 5;"
+ // Use both IN and MIN_MAX runtime filter types so both are generated on the join key.
+ sql "set runtime_filter_type = 'IN_OR_BLOOM_FILTER,MIN_MAX';"
+ sql "set runtime_filter_wait_time_ms = 10000;"
+ sql "set runtime_filter_wait_infinitely = true;"
Review Comment:
`runtime_filter_wait_infinitely = true` makes the effective wait time fall back to the query execution timeout (see RuntimeFilterConsumer), so the preceding `runtime_filter_wait_time_ms = 10000` no longer bounds the wait and the test could stall much longer than intended if a runtime filter is not produced. Consider setting `runtime_filter_wait_infinitely=false` (or removing it) if the goal is to cap waits at 10s, or otherwise update the comments to reflect the longer wait behavior.
```suggestion
// Cap runtime filter wait at 10s for this test instead of falling back to the query timeout.
sql "set runtime_filter_wait_infinitely = false;"
```
##########
be/src/exec/operator/olap_scan_operator.cpp:
##########
@@ -396,6 +396,24 @@ Status OlapScanLocalState::_process_conjuncts(RuntimeState* state) {
return Status::OK();
}
RETURN_IF_ERROR(_build_key_ranges_and_filters());
+
+ // _build_key_ranges_and_filters may erase predicates from _slot_id_to_predicates
+ // that are covered by exact key ranges. Re-print PushDownPredicates to reflect the
+ // actual predicates that will reach the storage layer.
+ if (state->enable_profile()) {
+ fmt::memory_buffer buf;
+ for (const auto& [slot_id, predicates] : _slot_id_to_predicates) {
+ if (predicates.empty()) {
+ continue;
+ }
+ fmt::format_to(buf, "Slot ID: {}: [", slot_id);
+ for (const auto& predicate : predicates) {
+ fmt::format_to(buf, "{{{}}}, ", predicate->debug_string());
+ }
+ fmt::format_to(buf, "] ");
+ }
+ custom_profile()->add_info_string("PushDownPredicates", fmt::to_string(buf));
Review Comment:
This block duplicates the formatting logic already implemented in `ScanLocalState` (see `predicates_to_string` in `scan_operator.cpp`). Consider factoring that helper into a shared header/utility so predicate formatting stays consistent across scan operators and avoids future drift when the format needs to change.
```suggestion
custom_profile()->add_info_string(
"PushDownPredicates", predicates_to_string(_slot_id_to_predicates));
```
##########
regression-test/suites/correctness_p0/test_rf_in_list_not_erased_by_scope_range.groovy:
##########
@@ -0,0 +1,90 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// This test verifies that when both MINMAX and IN runtime filters target the same
+// key column, and the IN filter's value count exceeds max_pushdown_conditions_per_column,
+// the IN_LIST predicate is NOT incorrectly erased by the key range construction logic.
+// Regression test for the bug where _build_key_ranges_and_filters() erased IN_LIST
+// predicates when the ColumnValueRange was a scope range (from MINMAX filter).
+suite("test_rf_in_list_not_erased_by_scope_range") {
+ sql "drop table if exists rf_scope_probe;"
+ sql "drop table if exists rf_scope_build;"
+
+ sql """
+ CREATE TABLE rf_scope_probe (
+ k1 BIGINT,
+ v1 INT
+ )
+ DUPLICATE KEY(k1)
+ DISTRIBUTED BY HASH(k1) BUCKETS 1
+ PROPERTIES ("replication_num" = "1");
+ """
+
+ sql """
+ CREATE TABLE rf_scope_build (
+ k1 BIGINT,
+ v1 INT
+ )
+ DUPLICATE KEY(k1)
+ DISTRIBUTED BY HASH(k1) BUCKETS 1
+ PROPERTIES ("replication_num" = "1");
+ """
+
+ // Probe table: insert 20 rows with k1 from 1 to 20.
+ // The build side will only match a subset (k1 in {2,4,6,8,10,12}).
+ // Rows NOT in this subset (k1=1,3,5,7,9,11,13..20) should be filtered out
+ // by the IN_LIST runtime filter.
+ sql """
+ INSERT INTO rf_scope_probe VALUES
+ (1, 1), (2, 2), (3, 3), (4, 4), (5, 5),
+ (6, 6), (7, 7), (8, 8), (9, 9), (10, 10),
+ (11, 11), (12, 12), (13, 13), (14, 14), (15, 15),
+ (16, 16), (17, 17), (18, 18), (19, 19), (20, 20);
+ """
+
+ // Build table: 6 distinct k1 values. This exceeds max_pushdown_conditions_per_column=5
+ // so the IN values are NOT added to ColumnValueRange, but the IN_LIST predicate is created.
+ // MINMAX range: [2, 12]
+ sql """
+ INSERT INTO rf_scope_build VALUES
+ (2, 100), (4, 200), (6, 300), (8, 400), (10, 500), (12, 600);
+ """
+
+ sql "sync;"
+
+ // Set max_pushdown_conditions_per_column to 5, so the 6 IN values exceed it.
+ // This causes IN values to NOT be added to the ColumnValueRange (it stays as
+ // a scope range from the MINMAX filter), but the IN_LIST ColumnPredicate is still created.
+ sql "set max_pushdown_conditions_per_column = 5;"
+ // Use both IN and MIN_MAX runtime filter types so both are generated on the join key.
+ sql "set runtime_filter_type = 'IN_OR_BLOOM_FILTER,MIN_MAX';"
+ sql "set runtime_filter_wait_time_ms = 10000;"
+ sql "set runtime_filter_wait_infinitely = true;"
+ sql "set enable_runtime_filter_prune = false;"
+ sql "set parallel_pipeline_task_num = 1;"
+
+ // The join should only return 6 rows (matching k1 in {2,4,6,8,10,12}).
+ // If the IN_LIST predicate is incorrectly erased, the MINMAX scope [2,12]
+ // would let through rows with k1 in {3,5,7,9,11} as well, producing wrong results.
+ // We verify correctness by checking the result.
+ order_qt_join """
+ SELECT p.k1, p.v1, b.k1, b.v1
+ FROM rf_scope_probe p
+ INNER JOIN rf_scope_build b ON p.k1 = b.k1
Review Comment:
This regression currently uses an INNER JOIN, so the result set is determined by the join condition (p.k1=b.k1) even if the runtime filter predicates are missing/erased; that means the test likely won’t fail under the bug described. To make it sensitive to an over-permissive runtime filter (e.g., MINMAX scope without the IN_LIST), consider using a LEFT SEMI JOIN / EXISTS pattern and enabling `enable_left_semi_direct_return_opt` (as other runtime-filter correctness tests do), so false positives like k1=3,5,7,9,11 would surface in the output when IN_LIST is erased.
```suggestion
sql "set enable_left_semi_direct_return_opt = true;"
// The join should only return 6 rows (matching k1 in {2,4,6,8,10,12}).
// If the IN_LIST predicate is incorrectly erased, the MINMAX scope [2,12]
// would let through rows with k1 in {3,5,7,9,11} as well, producing wrong results.
// We verify correctness by checking the result.
order_qt_join """
SELECT p.k1, p.v1
FROM rf_scope_probe p
WHERE EXISTS (
SELECT 1
FROM rf_scope_build b
WHERE p.k1 = b.k1
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org