github-actions[bot] commented on code in PR #63389:
URL: https://github.com/apache/doris/pull/63389#discussion_r3379953222
##########
be/src/exprs/function/functions_comparison.h:
##########
@@ -20,30 +20,141 @@
#pragma once
+#include <compare>
#include <limits>
+#include <optional>
+#include <string_view>
#include <type_traits>
+#include "common/check.h"
#include "common/logging.h"
#include "core/accurate_comparison.h"
#include "core/assert_cast.h"
#include "core/column/column_const.h"
#include "core/column/column_decimal.h"
#include "core/column/column_nullable.h"
#include "core/column/column_string.h"
+#include "core/data_type/data_type_nullable.h"
#include "core/data_type/data_type_number.h"
#include "core/data_type/data_type_string.h"
#include "core/data_type/define_primitive_type.h"
#include "core/decimal_comparison.h"
#include "core/field.h"
#include "core/memcmp_small.h"
#include "core/value/vdatetime_value.h"
+#include "exprs/expr_zonemap_filter.h"
#include "exprs/function/function.h"
#include "exprs/function/function_helpers.h"
#include "exprs/function/functions_logical.h"
+#include "exprs/vexpr.h"
#include "storage/index/index_reader_helper.h"
namespace doris {
+namespace comparison_zonemap_detail {
+
+enum class Op {
+ EQ,
+ NE,
+ LT,
+ LE,
+ GT,
+ GE,
+};
+
+inline Op symmetric_op(Op op) {
+ switch (op) {
+ case Op::EQ:
+ case Op::NE:
+ return op;
+ case Op::LT:
+ return Op::GT;
+ case Op::LE:
+ return Op::GE;
+ case Op::GT:
+ return Op::LT;
+ case Op::GE:
+ return Op::LE;
+ }
+ __builtin_unreachable();
+}
+
+inline ZoneMapFilterResult evaluate(const ZoneMapEvalContext& ctx, const
VExprSPtrs& arguments,
+ Op op) {
+ auto slot_literal = expr_zonemap::extract_slot_and_literal(arguments);
+ DORIS_CHECK(slot_literal.has_value());
+ DORIS_CHECK(slot_literal->slot_type != nullptr);
+ DORIS_CHECK(slot_literal->literal_type != nullptr);
+ DORIS_CHECK(!slot_literal->literal.is_null());
+ DORIS_CHECK_EQ(expr_zonemap::data_types_compatible(slot_literal->slot_type,
+
slot_literal->literal_type),
+ true)
+ << "slot type: " << slot_literal->slot_type->get_name()
+ << ", literal type: " << slot_literal->literal_type->get_name();
+
+ auto slot_type = expr_zonemap::fetch_compatible_slot_type(ctx,
slot_literal->slot_index,
+
slot_literal->slot_type);
+ if (slot_type == nullptr) {
+ return ZoneMapFilterResult::kUnsupported;
+ }
+ const auto* zone_map_ref = expr_zonemap::fetch_zone_map(ctx,
slot_literal->slot_index);
+ if (zone_map_ref == nullptr) {
+ return ZoneMapFilterResult::kUnsupported;
+ }
+ const auto& zone_map = *zone_map_ref;
+ if (!zone_map.has_not_null) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ if (!expr_zonemap::range_stats_usable_for_zonemap(zone_map, slot_type)) {
+ return unsupported_zonemap_filter(ctx);
+ }
+
+ const auto effective_op = slot_literal->literal_on_left ? symmetric_op(op)
: op;
+ const auto& literal = slot_literal->literal;
+ switch (effective_op) {
+ case Op::EQ:
+ return expr_zonemap::field_less(literal, zone_map.min_value) ||
+ expr_zonemap::field_greater(literal,
zone_map.max_value)
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ case Op::NE:
+ return zone_map.min_value == literal && zone_map.max_value == literal
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ case Op::LT:
+ return expr_zonemap::field_greater_equal(zone_map.min_value, literal)
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ case Op::LE:
+ return expr_zonemap::field_greater(zone_map.min_value, literal)
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ case Op::GT:
+ return expr_zonemap::field_less_equal(zone_map.max_value, literal)
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ case Op::GE:
+ return expr_zonemap::field_less(zone_map.max_value, literal)
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+ }
+ __builtin_unreachable();
+}
+
+inline bool can_evaluate(const VExprSPtrs& arguments) {
+ auto slot_literal = expr_zonemap::extract_slot_and_literal(arguments);
+ if (!slot_literal.has_value()) {
+ return false;
+ }
+
+ // A NULL literal makes the comparison evaluate to NULL instead of a byte
range
+ // predicate on the slot. This zonemap evaluator only derives bounds from
non-NULL
+ // literals, and evaluate_zonemap_filter asserts that precondition with
DORIS_CHECK.
+ return !slot_literal->literal.is_null();
+}
Review Comment:
`can_evaluate()` currently advertises zonemap support for any slot/literal
comparison as long as the literal is non-NULL, but `evaluate()` immediately
`DORIS_CHECK`s `data_types_compatible()` before it can fall back. If a
pushed-down BE comparison has a materialized literal type that is not
compatible with the slot type, this path is selected for expr-zonemap pruning
and the scanner aborts instead of conservatively returning
unsupported/may-match. This is distinct from the existing IN-list compatibility
thread because this is the comparison-function capability gate. Please include
the same compatibility check in `can_evaluate()` or make `evaluate()` return
`kUnsupported` instead of asserting for this fallback case, and add a unit test
for the mismatch.
--
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]
