github-actions[bot] commented on code in PR #65446:
URL: https://github.com/apache/doris/pull/65446#discussion_r3566178640
##########
be/src/format_v2/orc/orc_search_argument.cpp:
##########
@@ -671,11 +671,10 @@ std::optional<::orc::Literal> make_orc_literal(const
OrcSargColumn& sarg_column,
return make_date_literal(field);
case ::orc::PredicateDataType::TIMESTAMP: {
DORIS_CHECK(sarg_column.orc_type != nullptr);
- static const cctz::time_zone utc0 = cctz::utc_time_zone();
- const auto& literal_timezone =
- sarg_column.orc_type->getKind() ==
::orc::TypeKind::TIMESTAMP_INSTANT ? timezone
-
: utc0;
- return make_timestamp_literal(field, literal_timezone);
+ // Both plain TIMESTAMP and an unmapped TIMESTAMP_INSTANT are
materialized as DATETIMEV2
+ // with the session timezone. The SARG literal must use that same
timezone or stripe pruning
+ // can discard rows that Doris would decode as matches.
+ return make_timestamp_literal(field, timezone);
Review Comment:
This is still unsafe for named session timezones with a DST fall-back fold.
`make_timestamp_literal()` converts the local DATETIME literal to a single raw
instant, but row decode converts every raw ORC second back to local fields with
`from_unixtime(..., timezone)`. For example, in `America/Los_Angeles`, both
`2021-11-07 08:30:00Z` and `2021-11-07 09:30:00Z` decode to local `2021-11-07
01:30:00`; an equality, null-safe equality, single-value `IN`, or range SARG
built around only one `cctz::convert` result can prune a stripe containing only
the other instant before Doris row filtering runs. Please disable timestamp
SARGs for non-fixed-offset zones, or build transition-aware raw ranges that
cover every instant that can decode to the local predicate value/interval.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java:
##########
@@ -123,9 +132,25 @@ public void checkProperties() throws DdlException {
throw new DdlException(
"The parameter " + PARTITION_CACHE_TTL_SECOND + " is
wrong, value is " + partitionCacheTtlSecond);
}
+
+ getHiveParquetTimeZone();
catalogProperty.checkMetaStoreAndStorageProperties(AbstractHiveProperties.class);
}
+ public String getHiveParquetTimeZone() throws DdlException {
+ String hiveParquetTimeZone =
catalogProperty.getOrDefault(HIVE_PARQUET_TIME_ZONE, "").trim();
+ if (hiveParquetTimeZone.isEmpty()) {
+ return "";
+ }
+ try {
+ return
TimeUtils.checkTimeZoneValidAndStandardize(hiveParquetTimeZone);
Review Comment:
This still lets FE accept timezone strings that BE cannot parse.
`checkTimeZoneValidAndStandardize()` validates Java short IDs through
`ZoneId.SHORT_IDS` and then returns the original value unchanged; e.g. `AET` is
accepted by FE as `Australia/Sydney`, so this getter returns `AET` and
`FileQueryScanNode` sends that raw string to BE. BE's
`TimezoneUtils::find_cctz_time_zone()` does not have Java's short-id map, only
tzdata names plus UTC/GMT/offset normalization and the `z`/`cst` special cases,
so scans for such a catalog fail later with `Invalid hive.parquet.time-zone`.
Please either reject short aliases for this property or canonicalize them to a
BE-supported IANA/offset string before serializing the scan param.
--
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]
