github-actions[bot] commented on code in PR #65497:
URL: https://github.com/apache/doris/pull/65497#discussion_r3565838550
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -531,6 +532,11 @@ bool set_timestamp_zone_map(const ::orc::ColumnStatistics&
statistics,
timestamp_statistics->getMaximum(),
timestamp_statistics->getMaximumNanos()));
return true;
}
+ if (!format::utc_timestamp_range_is_monotonic(
Review Comment:
This only disables the ZoneMap conversion path, but ORC scan pruning still
builds timestamp SearchArguments in
`_init_search_argument_from_local_filters()` and uses `getNeedReadStripes()`
before row-level filters run. For a `TIMESTAMP_INSTANT` column decoded as
session-local `DATETIMEV2`, a rollback value such as `2021-11-07 01:30` in
`America/New_York` names two UTC instants; the SARG literal conversion picks a
single timestamp and lets ORC compare it to stripe min/max, so a stripe
containing the other repeated-hour instant can still be pruned. Please also
guard or disable timestamp SARG pruning for non-monotonic local mappings, and
add an ORC scan test for that rollback case.
##########
be/src/format_v2/parquet/parquet_statistics.cpp:
##########
@@ -118,13 +120,48 @@ bool set_decoded_field(const ParquetColumnSchema&
column_schema, DecodedValueKin
return read_decoded_field(column_schema, view, field, timezone).ok();
}
+int64_t floor_timestamp_seconds(int64_t value, ParquetTimeUnit time_unit) {
+ int64_t units_per_second = 1;
+ switch (time_unit) {
+ case ParquetTimeUnit::MILLIS:
+ units_per_second = 1000;
+ break;
+ case ParquetTimeUnit::MICROS:
+ units_per_second = 1000000;
+ break;
+ case ParquetTimeUnit::NANOS:
+ units_per_second = 1000000000;
+ break;
+ default:
+ DORIS_CHECK(false);
+ }
+ return format::floor_epoch_seconds(value, units_per_second);
+}
+
+bool timestamp_min_max_is_safe(const ParquetColumnSchema& column_schema,
int64_t min_value,
+ int64_t max_value, const cctz::time_zone*
timezone) {
+ if (!column_schema.type_descriptor.is_timestamp ||
Review Comment:
This also disables min/max for adjusted-to-UTC Parquet timestamps after
`enable_mapping_timestamp_tz` maps the column to `TYPE_TIMESTAMPTZ`. In that
mode the decoded values are `TimestampTzValue`s stored and compared in UTC, so
the local `DATETIMEV2` monotonicity problem does not apply; ORC handles the
analogous path by returning `TYPE_TIMESTAMPTZ` min/max before running this
guard. Since `apply_timestamp_tz_mapping()` leaves the Parquet timestamp flags
set, rollback-spanning row groups/pages lose safe pruning and Parquet min/max
aggregate pushdown returns `NotSupported`. Please skip this guard when the
effective Doris type is `TYPE_TIMESTAMPTZ`, and add coverage for that config
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]
