Gabriel39 commented on code in PR #69646:
URL: https://github.com/apache/doris/pull/69646#discussion_r4385379315
##########
be/src/exprs/vtopn_pred.cpp:
##########
@@ -97,6 +97,42 @@ bool topn_string_matches(int comparison, RawComparisonOp op)
{
} // namespace
+bool VTopNPred::can_evaluate_zonemap_filter() const {
+ // Capability must remain available before the first bound; readers cache
it during open.
+ return _predicate != nullptr && _children.size() == 1 &&
_children[0]->is_slot_ref();
+}
+
+ZoneMapFilterResult VTopNPred::evaluate_zonemap_filter(const
ZoneMapEvalContext& ctx) const {
+ if (!can_evaluate_zonemap_filter()) {
+ return unsupported_zonemap_filter(ctx);
+ }
+ const Field bound = _predicate->get_value();
+ if (bound.is_null()) {
+ return ZoneMapFilterResult::kMayMatch;
+ }
+ const auto* slot = assert_cast<const VSlotRef*>(_children[0].get());
+ const auto zone = ctx.zone_map(slot->column_id());
+ const auto type = ctx.data_type(slot->column_id());
+ if (zone == nullptr || !expr_zonemap::data_types_compatible(type,
slot->data_type())) {
+ return unsupported_zonemap_filter(ctx);
+ }
+ if (_predicate->nulls_first() && zone->has_null) {
+ return ZoneMapFilterResult::kMayMatch;
+ }
+ if (!zone->has_not_null) {
+ return ZoneMapFilterResult::kNoMatch;
+ }
+ if (ctx.floating_nan_count_unknown(slot->column_id()) || bound.is_nan() ||
+ !expr_zonemap::range_stats_usable_for_zonemap(*zone, type) ||
Review Comment:
Confirmed and fixed. The shared TopN evaluator now validates the bound and
both min/max Field carriers against the slot type before calling the asserting
typed-range helper. Incompatible metadata returns the existing unsupported
result and retains the candidate. The shared helper's invariant and reader
routing remain unchanged.
The regression calls the actual retained V1 BYTE_ARRAY statistics decoder
for a VARBINARY schema and verifies that it produces STRING Fields. With no
bound it retains the group; publishing the real runtime bound reproduced the
reported assertion on the previous head. After the fix, ASC/DESC and either
mixed min/max carrier fall back, while correctly typed VARBINARY controls still
reject strictly worse ranges and retain equality. Binary samples contain
embedded NUL/high bytes. This is a decoder-to-runtime-predicate integration
regression, not a full rolling-upgrade cluster test.
The expanded BE run passes 498 tests, with the opt-in reader benchmark
skipped. FE sources are unchanged by this follow-up; the 30 FE tests, including
all three translated aggregation EXPLAIN cases, remain verified by the
preceding revision.
--
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]
