github-actions[bot] commented on code in PR #69646:
URL: https://github.com/apache/doris/pull/69646#discussion_r4385250260
##########
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:
[P2] Reject legacy VARBINARY statistic carriers before the range check
During a BE-first rolling upgrade, an older FE request can omit
`parquet_timestamp_semantics_version` and retain V1 Parquet with
`enable_file_scanner_v2` false/unset and no `hive_parquet_time_zone`. Consider
`TopN(b ASC LIMIT 1) -> FileScan(b VARBINARY)` with binary mapping and
expression ZoneMap filtering enabled. Once an earlier group/scanner publishes a
non-NULL bound, the next V1 group reaches this new evaluator.
`ParquetPredicate::parse_min_max_value()` builds a `ColumnString` for
unannotated BYTE_ARRAY and keeps it through `ConsistentPhysicalConverter`,
producing TYPE_STRING bounds under a TYPE_VARBINARY context. The file/table
type checks pass, but `range_stats_usable_for_zonemap()` DORIS_CHECKs the bound
Field tags before this function can fall back, failing a valid query in Release
and aborting in Debug. The existing exclusion in `functions_comparison.h`
documents the same carrier mismatch. Previously TopN did not advertise this
capability. Validate the zone's min/max Fie
ld types before the asserting helper and return unsupported, or normalize the
carrier; add a late-bound legacy VARBINARY regression. Current FE plans force
V2, so the affected path is the explicitly retained legacy-request
compatibility path.
--
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]
