github-actions[bot] commented on code in PR #67774:
URL: https://github.com/apache/doris/pull/67774#discussion_r4104417300
##########
regression-test/suites/query_p0/expr_zonemap/test_expr_zonemap_pruning.groovy:
##########
@@ -329,4 +324,333 @@ suite("test_expr_zonemap_pruning") {
"""
assertEquals(0L, isNotNullPrunedRows[0][1] as long)
assertExprZonemapPruned(isNotNullToken)
+
+ // Column-vs-column comparisons. A predicate over two columns of the same
table never became a
+ // ColumnPredicate, so it reaches the scanner as a common expression and
is evaluated against the
+ // segment zone map of both slots at once.
+ sql """ DROP TABLE IF EXISTS test_expr_zonemap_pruning_two_columns """
+ sql """
+ CREATE TABLE test_expr_zonemap_pruning_two_columns (
+ id INT,
+ lo INT,
+ hi INT,
+ alt INT,
+ expected INT,
+ actual INT
+ ) ENGINE=OLAP
+ DUPLICATE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1",
+ "disable_auto_compaction" = "true"
+ )
+ """
+ // lo lands in [0, 4095] and hi in [10000, 14095], so the two ranges are
fully separated. alt
+ // lands in [0, 7095] and equals lo on even rows and lo + 3000 on odd
ones, so lo vs alt cannot
+ // be decided from the bounds and every row has to be evaluated. expected
and actual are both
+ // the constant 7.
+ sql """
+ INSERT INTO test_expr_zonemap_pruning_two_columns
+ SELECT CAST(number AS INT),
+ CAST(number AS INT),
+ CAST(number + 10000 AS INT),
+ IF(number % 2 = 0, CAST(number AS INT), CAST(number + 3000 AS
INT)),
+ 7,
+ 7
+ FROM numbers("number" = "4096")
+ """
+ sql """ sync """
+
+ // Runs the same query with expr zonemap pruning on and off and asserts
the two agree. The
+ // counter only shows that pruning fired; this is what shows it fired
correctly.
+ def assertSameWithAndWithoutPruning = { String predicate ->
+ sql """ set enable_expr_zonemap_filter = false """
+ def withoutPruning = sql """
+ SELECT COUNT(*) FROM test_expr_zonemap_pruning_two_columns WHERE
${predicate}
+ """
+ sql """ set enable_expr_zonemap_filter = true """
+ def withPruning = sql """
+ SELECT COUNT(*) FROM test_expr_zonemap_pruning_two_columns WHERE
${predicate}
+ """
+ assertEquals(withoutPruning[0][0] as long, withPruning[0][0] as long)
+ return withPruning[0][0] as long
+ }
+
+ def assertTwoColumnPruned = { String predicate, String label ->
+ def token = "expr_zonemap_pruning_two_columns_" + label + "_" +
UUID.randomUUID().toString()
+ def rows = sql """
+ SELECT '${token}', COUNT(*) FROM
test_expr_zonemap_pruning_two_columns
+ WHERE ${predicate}
+ """
+ assertEquals(0L, rows[0][1] as long)
+ assertExprZonemapPruned(token)
+ assertEquals(0L, assertSameWithAndWithoutPruning(predicate))
+ }
+
+ // lo > hi and lo >= hi: rejected because min(hi) is already above max(lo).
+ assertTwoColumnPruned("lo > hi", "gt")
+ assertTwoColumnPruned("lo >= hi", "ge")
+ // hi < lo and hi <= lo: the mirrored rules.
+ assertTwoColumnPruned("hi < lo", "lt")
+ assertTwoColumnPruned("hi <= lo", "le")
+ // lo = hi: the ranges are disjoint, so no row can be equal.
+ assertTwoColumnPruned("lo = hi", "eq")
+ // expected != actual: both columns collapse to the single value 7, which
is the only shape that
+ // lets != prune.
+ assertTwoColumnPruned("expected != actual", "ne")
+
+ // Overlapping ranges must survive, and the row counts must be exact.
These are the cases that
+ // catch a rule reading the wrong end of a range: lo in [0, 4095] against
alt in [0, 7095] cannot
+ // be separated by the bounds, so all of these have to fall through to
per-row evaluation.
+ assertEquals(2048L, assertSameWithAndWithoutPruning("lo < alt"))
Review Comment:
[P2] Record fixed SQL results through the golden-output path
The added block encodes predetermined query counts directly with
`assertEquals` here and at the other fixed-result checks. Root `AGENTS.md`
requires determined regression results to use named `qt_*`/`order_qt_*` queries
and runner-generated `.out` output. The enabled-vs-disabled equality and
profile-counter checks are dynamic invariants and can stay programmatic, but
please move the fixed row-count expectations into the golden-output path.
##########
be/src/format/parquet/vparquet_reader.cpp:
##########
@@ -1648,11 +1653,22 @@ Status
ParquetReader::_process_expr_zonemap_filter(const tparquet::RowGroup& row
return Status::OK();
}
+ // The v1 Parquet reader is being removed, so it does not take on the new
slot-vs-slot pruning.
+ // Restrict this path to single-slot conjuncts, mirroring the page path's
single-column
+ // requirement above; a two-slot shape falls back to no pruning here.
Native and v2 keep it.
std::set<int> column_ids;
+ VExprContextSPtrs single_slot_conjuncts;
for (const auto& conjunct : all_conjuncts) {
- if (conjunct->root() != nullptr &&
conjunct->root()->can_evaluate_zonemap_filter()) {
- conjunct->root()->collect_slot_column_ids(column_ids);
+ if (conjunct->root() == nullptr ||
!conjunct->root()->can_evaluate_zonemap_filter()) {
+ continue;
+ }
+ std::set<int> conjunct_column_ids;
+ conjunct->root()->collect_slot_column_ids(conjunct_column_ids);
+ if (conjunct_column_ids.size() != 1) {
Review Comment:
[P1] Reject slot-slot leaves, not just distinct ordinals
This new fence counts unique block ordinals, so a comparison with two
`VSlotRef`s for the same column still has size one. For example, the supported
`enable_file_scanner_v2=false` and
`disable_nereids_expression_rules=SIMPLIFY_SELF_COMPARISON` settings let `a <
a` reach BE in that shape. This row-group path and the identical
`_expr_zonemap_page_slot_index` test then parse v1 fixed-width statistics
before evaluation, so a short bound can still be read before fallback. This is
the residual current-head failure of the mitigation claimed in
`discussion_r4060255032`. Please reject every slot-vs-slot leaf in both v1
gates, including equal-ordinal leaves nested under AND/OR; checking only
`extract_slot_and_slot` on the root would miss those compounds.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
