LuciferYang commented on code in PR #67774:
URL: https://github.com/apache/doris/pull/67774#discussion_r4043634648


##########
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:
   Adopted, thanks — the length check preserves short-string pruning and keeps 
the existing 'a'/'b' fixture, done in e65bcdfc634.
   
   `can_evaluate_slot_slot` now allows string/char/varchar pairs again; 
`evaluate_slot_slot` adds the four-bound check after the all-NULL and 
statistics-validity checks: every bound must have `.size() < 
MAX_ZONE_MAP_INDEX_SIZE`, otherwise it falls back. As you noted, the writer's 
last-byte increment doesn't change the length, so a 512-byte bound is excluded 
whether or not it wrapped, with no truncation flag. `Field::get<TYPE_STRING>()` 
reads CHAR/VARCHAR/STRING alike, so one length path covers all three.
   
   `SlotSlotStringPrunesUntruncatedBoundsOnly` covers 511 (prunes), 512 and 513 
(fall back), a 512-byte bound ending in 0xff (falls back), and an embedded-NUL 
short bound (prunes, since NUL counts toward size); mutation-checked by 
neutering the length guard. The native writer/reader end-to-end test you'd 
prefer is not in this commit; the unit test drives `evaluate_slot_slot` bounds 
directly.



-- 
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]

Reply via email to