github-actions[bot] commented on code in PR #67774:
URL: https://github.com/apache/doris/pull/67774#discussion_r4033245012
##########
be/src/format/parquet/vparquet_reader.cpp:
##########
@@ -1665,14 +1670,22 @@ Status
ParquetReader::_process_expr_zonemap_filter(const tparquet::RowGroup& row
}
auto* slot = _tuple_descriptor->slots()[cid];
ZoneMapEvalContext::SlotZoneMap slot_zone_map;
- slot_zone_map.data_type = slot->type();
+ slot_zone_map.set_data_type_from_parquet(slot->type());
if (!_exists_in_file(slot->col_name()) || !_type_matches(cid)) {
ctx.slots.emplace(cid, std::move(slot_zone_map));
continue;
}
const auto& file_col_name =
_table_info_node_ptr->children_file_column_name(slot->col_name());
const FieldSchema* col_schema =
_file_metadata->schema().get_column(file_col_name);
+ // parse_min_max_value decodes the bounds in the file's own logical
type, while _type_matches
+ // only compares primitive types. A DECIMAL bound decoded at the
file's scale would then be
+ // compared against the table's scale as if the payloads shared a
domain, so leave the zone
+ // map out unless the two types agree exactly.
Review Comment:
[P1] Reject v1 timestamp ranges that cross a clock rollback
This exact-type guard still admits adjusted-UTC Parquet TIMESTAMP mapped to
DATETIMEV2. V1 converts only the physical min/max into local civil time, which
is not monotonic over DST fallback: in America/New_York UTC `[05:30, 06:30]`
converts to `[01:30, 01:30]` although an interior 05:59 row becomes 01:59.
There is no raw-range rollback fence; the converted DATETIMEV2 fallback also
reads both `min_value` and `max_value` from `min_field`, and even fixing that
typo would miss endpoints that collapse to the same civil value. Paired with a
constant DATETIMEV2 column at 01:45, the new `a > b` proof prunes even though
the 01:59 row satisfies it. V2 already uses
`utc_timestamp_range_is_monotonic()` here. Please add the same raw-range fence
to v1 (or reject nonpoint physical timestamp stats) and a v1 DST-rollback
two-column test.
##########
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:
[P1] Reject wrapped string zone-map maxima before slot-slot pruning
This gate admits STRING/VARCHAR pairs, but native zone maps truncate a max
to 512 bytes and `modify_index_before_flush()` increments byte 512 without
carry. SQL can store arbitrary bytes via `unhex`, so a left value `prefix(511)
|| 0xff` is serialized with min ending `ff` and max ending `00`; for a right
value ending `80`, `a > b` is true row-wise but `range_vs_range_no_match()`
sees `rmin >= lmax` and drops the segment. This two-slot false negative is
newly reachable through this gate. Please either make persisted/native
truncated maxima conservatively unusable (including old wrapped maps) or
exclude string pairs here, and add a binary-boundary regression.
--
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]
