github-actions[bot] commented on code in PR #67774:
URL: https://github.com/apache/doris/pull/67774#discussion_r4046434599
##########
be/src/exprs/function/functions_comparison.h:
##########
@@ -314,42 +342,94 @@ inline ZoneMapFilterResult evaluate(const
ZoneMapEvalContext& ctx, const VExprSP
return unsupported_zonemap_filter(ctx);
}
- const auto effective_op = slot_literal->literal_on_left ? symmetric_op(op)
: op;
- const auto& literal = slot_literal->literal;
+ const auto effective_op = slot_literal.literal_on_left ? symmetric_op(op)
: op;
+ const auto& literal = slot_literal.literal;
const bool literal_is_nan = literal.is_nan();
const bool hidden_nan_can_match = (effective_op == Op::EQ &&
literal_is_nan) ||
(effective_op == Op::NE &&
!literal_is_nan) ||
(effective_op == Op::GT &&
!literal_is_nan) ||
effective_op == Op::GE;
- if (ctx.floating_nan_count_unknown(slot_literal->slot_index) &&
hidden_nan_can_match) {
+ if (ctx.floating_nan_count_unknown(slot_literal.slot_index) &&
hidden_nan_can_match) {
// Parquet bounds omit NaNs, so only operators that cannot match a
hidden NaN may prune.
return unsupported_zonemap_filter(ctx);
}
- switch (effective_op) {
- case Op::EQ:
- return literal < zone_map.min_value || zone_map.max_value < literal
- ? 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 zone_map.min_value >= literal ? ZoneMapFilterResult::kNoMatch
- : ZoneMapFilterResult::kMayMatch;
- case Op::LE:
- return zone_map.min_value > literal ? ZoneMapFilterResult::kNoMatch
- : ZoneMapFilterResult::kMayMatch;
- case Op::GT:
- return zone_map.max_value <= literal ? ZoneMapFilterResult::kNoMatch
- : ZoneMapFilterResult::kMayMatch;
- case Op::GE:
- return zone_map.max_value < literal ? ZoneMapFilterResult::kNoMatch
- : ZoneMapFilterResult::kMayMatch;
+ return range_vs_range_no_match(zone_map.min_value, zone_map.max_value,
literal, literal,
+ effective_op)
+ ? ZoneMapFilterResult::kNoMatch
+ : ZoneMapFilterResult::kMayMatch;
+}
+
+inline ZoneMapFilterResult evaluate_slot_slot(const ZoneMapEvalContext& ctx,
+ const expr_zonemap::SlotSlot&
slot_slot, Op op) {
+ const auto left_type = expr_zonemap::fetch_compatible_slot_type(ctx,
slot_slot.left_slot_index,
+
slot_slot.left_type);
+ const auto right_type = expr_zonemap::fetch_compatible_slot_type(
+ ctx, slot_slot.right_slot_index, slot_slot.right_type);
+ if (left_type == nullptr || right_type == nullptr) {
+ // The context skips a slot entirely when the segment cannot apply
predicates on it.
+ return unsupported_zonemap_filter(ctx);
}
+ const auto left_zone_map = ctx.zone_map(slot_slot.left_slot_index);
+ const auto right_zone_map = ctx.zone_map(slot_slot.right_slot_index);
+ if (left_zone_map == nullptr || right_zone_map == nullptr) {
+ // A slot can be present with a data type but no zone map, so this is
a separate check.
+ return unsupported_zonemap_filter(ctx);
+ }
+ // A column holding no non-null value makes the comparison NULL on every
row, which never
+ // satisfies a WHERE conjunct. This must run before the range checks
below: an all-null zone map
+ // leaves min/max default-constructed as TYPE_NULL, which the range checks
would fatal on.
+ if (!left_zone_map->has_not_null || !right_zone_map->has_not_null) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ if (!expr_zonemap::range_stats_usable_for_zonemap(*left_zone_map,
left_type) ||
+ !expr_zonemap::range_stats_usable_for_zonemap(*right_zone_map,
right_type)) {
+ return unsupported_zonemap_filter(ctx);
+ }
+ // A native string zone-map max is truncated to MAX_ZONE_MAP_INDEX_SIZE,
then its last byte is
+ // incremented (which wraps on 0xff), so a bound of exactly that length is
not a reliable fence
+ // for a two-sided proof. A shorter bound is the untruncated exact value;
the increment never
+ // changes the length, so this also rejects an already-wrapped bound
without a truncation flag.
+ // The test only decides whether to trust the bound as a fence. A
Parquet-sourced string bound
+ // shorter than the cap is not necessarily exact, but it is a
spec-conservative min/max, which
+ // is all a range proof needs; do not tighten this to assume exactness
below the cap.
+ // data_types_compatible pairs strings only with strings, so checking one
side's type is enough
+ // to know all four bounds are strings.
+ if (is_string_type(remove_nullable(left_type)->get_primitive_type())) {
+ auto is_untruncated = [](const Field& f) {
+ return f.get<TYPE_STRING>().size() < MAX_ZONE_MAP_INDEX_SIZE;
+ };
+ if (!is_untruncated(left_zone_map->min_value) ||
+ !is_untruncated(left_zone_map->max_value) ||
+ !is_untruncated(right_zone_map->min_value) ||
+ !is_untruncated(right_zone_map->max_value)) {
+ return unsupported_zonemap_filter(ctx);
+ }
+ }
+ // Parquet bounds omit NaN without recording how many were skipped, so a
zone map that looks
+ // like a single point may still hide one, which would flip the NE rule
from false to true. The
+ // slot-vs-literal path can reason per operator because it knows whether
the literal is NaN;
+ // with two slots there is no literal, so bail out for every operator
instead.
+ if (ctx.floating_nan_count_unknown(slot_slot.left_slot_index) ||
+ ctx.floating_nan_count_unknown(slot_slot.right_slot_index)) {
+ return unsupported_zonemap_filter(ctx);
+ }
+ return range_vs_range_no_match(left_zone_map->min_value,
left_zone_map->max_value,
Review Comment:
[P2] Prune same-slot `!=` without requiring a point range
This new test path leaves `a != a` as `kMayMatch` whenever the slot spans
more than one value, but Nereids' `SimplifySelfComparison` does not fold
`NotEqualTo`, so the predicate reaches BE. For every row, a non-NULL value is
equal to itself (including Doris NaN semantics) and NULL yields UNKNOWN, so a
WHERE conjunct can never match; on a range-valued segment the current
independent-range rule still forces the scan. Please detect that both operands
are the same logical slot before applying the generic range-vs-range proof and
return `kNoMatch` for NE. Use logical slot identity rather than only the
possibly collapsed column ordinal, and make the range-valued same-slot test
expect pruning. If the fix uses `slot_id`, also initialize stable logical IDs
in the BE unit helper, which currently sets only `column_id`.
--
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]
