LuciferYang commented on code in PR #67774:
URL: https://github.com/apache/doris/pull/67774#discussion_r4110664312
##########
be/src/exprs/expr_zonemap_filter.cpp:
##########
@@ -250,6 +250,39 @@ std::optional<SlotLiteral> extract_slot_and_literal(const
VExprSPtrs& args) {
return std::nullopt;
}
+std::optional<SlotSlot> extract_slot_and_slot(const VExprSPtrs& args) {
+ if (args.size() != 2) {
+ return std::nullopt;
+ }
+ auto left = std::dynamic_pointer_cast<VSlotRef>(args[0]);
+ auto right = std::dynamic_pointer_cast<VSlotRef>(args[1]);
+ if (left == nullptr || right == nullptr) {
+ return std::nullopt;
+ }
+ return SlotSlot {.left_slot_index = left->column_id(),
+ .left_type = left->data_type(),
+ .right_slot_index = right->column_id(),
+ .right_type = right->data_type()};
+}
+
+bool contains_slot_slot_comparison(const VExprSPtr& expr) {
+ if (expr == nullptr) {
+ return false;
+ }
+ // A comparison node's two operands are its children, so a slot-vs-slot
leaf is a node whose
+ // children are two VSlotRef. extract_slot_and_slot accepts a same-column
pair too, which is the
+ // point: `a < a` has one column id but is still slot-vs-slot.
+ if (extract_slot_and_slot(expr->children()).has_value()) {
Review Comment:
Good catch that the detector matches any two-slot binary node, `add(a, a)`
included, not just comparison functions. It is used only in the two v1 Parquet
gates (`contains_slot_slot_comparison` appears nowhere else under `be/src`), so
the effect is confined to v1: the conjunct is skipped for v1 pruning. That
costs an optimization, not correctness, since skipping returns a superset and
never wrong rows. The v1 reader is being removed, so I'm keeping the
conservative fence rather than narrowing it to comparison nodes. The v2/native
path doesn't go through this detector.
##########
be/src/format/parquet/vparquet_reader.cpp:
##########
@@ -1648,11 +1658,24 @@ 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-column conjuncts that carry no
slot-vs-slot leaf: counting
+ // distinct columns alone is not enough, because a same-column pair like
`a < a` still has one
+ // column id. 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:
Same trade-off: the fence drops v1 row-group pruning for multi-column
compounds that carry no slot-slot leaf. It costs pruning, not correctness (the
group is read rather than skipped, so the result stays a superset). With the v1
reader being removed, I'm not extending the fence to keep that v1 pruning alive.
--
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]
