github-actions[bot] commented on code in PR #65446:
URL: https://github.com/apache/doris/pull/65446#discussion_r3557691146
##########
be/src/format_v2/parquet/reader/parquet_leaf_reader.cpp:
##########
@@ -394,14 +402,16 @@ ParquetLeafReader::ParquetLeafReader(
std::shared_ptr<::parquet::internal::RecordReader> record_reader,
ParquetColumnReaderProfile profile, const cctz::time_zone* timezone,
bool enable_strict_mode,
- std::function<Status(MutableColumnPtr&, const DecodedColumnView&)>
decoded_value_appender)
+ std::function<Status(MutableColumnPtr&, const DecodedColumnView&)>
decoded_value_appender,
+ const cctz::time_zone* int96_timezone)
Review Comment:
Adding this `int96_timezone` parameter is not enough unless every
`ParquetLeafReader` construction that materializes values forwards it.
`ScalarColumnReader::leaf_reader()` passes `_int96_timezone` into the outer
leaf reader, but `build_nested_batch_from_leaf_batch()` later creates a second
`value_reader` for nested scalar payloads without this final argument. For
`ARRAY`/`STRUCT`/`MAP` leaves encoded as INT96 `TIMESTAMP`, `append_values()`
then sees `DecodedValueKind::INT96` with a null INT96 timezone and falls back
to the default raw/UTC behavior, while top-level INT96 columns use the
configured `hive.parquet.time-zone`. Please pass `_int96_timezone` into that
inner reader too, and add a nested INT96 test so the catalog timezone is
exercised below a complex type.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java:
##########
@@ -123,9 +132,24 @@ public void checkProperties() throws DdlException {
throw new DdlException(
"The parameter " + PARTITION_CACHE_TTL_SECOND + " is
wrong, value is " + partitionCacheTtlSecond);
}
+
+ String hiveParquetTimeZone = getHiveParquetTimeZone();
+ if (!hiveParquetTimeZone.isEmpty()) {
+ try {
+
TimeUtils.checkTimeZoneValidAndStandardize(hiveParquetTimeZone);
Review Comment:
This validates shorthand offsets but then discards the canonical value.
`TimeUtils.checkTimeZoneValidAndStandardize("8:00")` returns `"+08:00"` and
existing `TimeUtilsTest` covers that behavior, yet `checkProperties()` ignores
the return value and `getHiveParquetTimeZone()` continues to return the raw
catalog property. `FileQueryScanNode` then serializes raw `"8:00"` to BE, where
`ParquetReader::init()` calls `TimezoneUtils::find_cctz_time_zone`; the BE
parser only normalizes `UTC`/`GMT` prefixes or values starting with `+`/`-`, so
the scan fails with `Invalid hive.parquet.time-zone: 8:00` even though catalog
validation accepted it. Please either persist/return the standardized value
before thrift serialization, or reject shorthand forms consistently, and add
coverage for a shorthand offset reaching the BE-facing scan params.
--
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]
