HappenLee commented on code in PR #67774:
URL: https://github.com/apache/doris/pull/67774#discussion_r4036925282
##########
be/src/exprs/function/functions_comparison.h:
##########
@@ -377,6 +436,24 @@ inline bool can_evaluate(const VExprSPtrs& arguments) {
return true;
}
+// Accept a comparison whose both operands are slot references. Kept separate
from can_evaluate on
+// purpose: can_evaluate also gates dictionary filtering and
can_evaluate_equality, and both of
+// those dereference extract_slot_and_literal behind a DORIS_CHECK, so
widening it would abort on a
+// slot-vs-slot expression. Only can_evaluate_zonemap_filter ORs this in.
+inline bool can_evaluate_slot_slot(const VExprSPtrs& arguments) {
+ auto slot_slot = expr_zonemap::extract_slot_and_slot(arguments);
+ if (!slot_slot.has_value()) {
+ return false;
+ }
+ DORIS_CHECK(slot_slot->left_type != nullptr);
+ DORIS_CHECK(slot_slot->right_type != nullptr);
+ // The two zone maps' Fields are compared directly and Field comparison
throws on mismatched
+ // non-string types, so reject incompatible column pairs here. A pair
differing only by width or
+ // decimal scale never reaches this point anyway, because the optimizer
inserts a cast and a
+ // cast is not a VSlotRef.
+ return expr_zonemap::data_types_compatible(slot_slot->left_type,
slot_slot->right_type);
Review Comment:
Could we preserve string slot-vs-slot pruning for short bounds by checking
the actual zone maps in `evaluate_slot_slot()`?
For the native STRING/VARCHAR truncation issue described here, a
conservative condition would be that all four bounds (`left.min`, `left.max`,
`right.min`, and `right.max`) have byte lengths strictly less than
`MAX_ZONE_MAP_INDEX_SIZE` (512). Truncation produces a 512-byte bound, and
incrementing the last byte does not change its length, even when `0xff` wraps
to `0x00`. This would also reject existing wrapped 512-byte bounds without
requiring a new truncation flag.
The check should use the stored string's `.size()`, not `strlen()` or the
declared VARCHAR width. Exactly 512 bytes must remain excluded because the
writer also increments the last byte at that length. Since
`can_evaluate_slot_slot()` has no zone-map values, the length check belongs in
evaluation after the existing all-NULL and statistics-validity checks, while
retaining the other type/producer safety checks.
This would allow the existing short-string per-segment fixture (`'a'`/`'b'`)
to keep testing the expected 2/1 filtered segments. Please also cover
511/512/513-byte bounds, a 512th byte of `0xff`, and embedded NUL bytes,
ideally through a native writer/reader test to verify that unsafe bounds fall
back without dropping matching rows.
--
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]
