github-actions[bot] commented on code in PR #65446:
URL: https://github.com/apache/doris/pull/65446#discussion_r3701947246
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -1162,8 +1162,8 @@ Status ParquetScanScheduler::open_next_row_group(
RETURN_IF_ERROR(NativeColumnReader::create(
*column_schema, &col, file_context.native_data_file(),
file_context.native_metadata,
row_group_idx, _current_selected_ranges,
_current_offset_indexes, _timezone,
- file_context.native_io_ctx, _runtime_state,
file_context.native_page_cache_enabled,
- file_context.native_page_cache_file_key,
+ _int96_timezone, file_context.native_io_ctx, _runtime_state,
Review Comment:
[P1] Preserve legacy INT96 semantics for old-FE plans
During a BE-first rolling upgrade, an old FE cannot set the new scan field,
so `_int96_timezone` is null here. Passing that raw pointer to the optional
parameter nevertheless creates an engaged optional, and the scalar reader
selects null/UTC raw-wall decoding; an old BE executing another range from the
same plan still uses the session timezone. Because external ranges are
distributed across mixed BEs without a semantic-version fence, one query can
return backend-dependent timestamp values. Please add an Iceberg-style
semantics marker/tri-state where absence preserves the legacy session path and
only an opted-in new plan selects raw or configured-zone behavior, including
Paimon field semantics, with an old-FE absent-field test.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java:
##########
@@ -244,6 +247,15 @@ public String getMetaCacheEngine() {
return "default";
}
+ public String getHiveParquetTimeZone() throws UserException {
+ if (catalog == null || !("hms".equalsIgnoreCase(catalog.getType())
Review Comment:
[P2] Route the configured timezone through the provider
The public capability is ineffective for any standalone connector type other
than the literal `hms`/`hudi` names: after its provider returns true, this
helper drops the configured value solely from `catalog.getType()`. The
connector SPI explicitly permits arbitrary standalone types, so a valid
`corp_hive` provider with `hive.parquet.time-zone=Asia/Shanghai` and this
capability enabled will omit field 35 and silently read its INT96 values with
raw-wall semantics. Please have the serving provider validate and
return/populate the timezone (the existing `populateScanLevelParams` hook is
one option), remove the generic type-name gate, and add an integration test
with a non-built-in provider type.
##########
be/src/core/data_type_serde/data_type_serde.cpp:
##########
@@ -477,6 +477,24 @@ int64_t find_struct_child_index(const ::orc::Type& type,
const std::string& fiel
return -1;
}
+struct RoundedOrcTimestamp {
+ int64_t seconds;
+ uint64_t microseconds;
+};
+
+RoundedOrcTimestamp round_orc_timestamp_to_microseconds(int64_t seconds,
int64_t nanoseconds) {
+ constexpr int64_t NANOS_PER_SECOND = 1000000000;
+ constexpr int64_t NANOS_PER_MICROSECOND = 1000;
+ constexpr int64_t MICROS_PER_SECOND = 1000000;
+ DORIS_CHECK(nanoseconds >= 0 && nanoseconds < NANOS_PER_SECOND);
+ // Doris stores six fractional digits, so use half-up rounding and carry
999999500ns into the
+ // next second instead of silently truncating the ORC value.
+ const auto rounded_microseconds =
+ (nanoseconds + NANOS_PER_MICROSECOND / 2) / NANOS_PER_MICROSECOND;
+ return {.seconds = seconds + rounded_microseconds / MICROS_PER_SECOND,
Review Comment:
[P1] Validate ORC rounding carry before conversion
A valid ORC value at `9999-12-31 23:59:59.999999500` makes this new addition
advance to the first second of year 10000. Both row decoders then call
`DateV2Value::from_unixtime`, which explicitly performs no range check, and
return OK with an invalid DATETIMEV2/TIMESTAMPTZ; the statistics helper mirrors
the same carry. An encoded `seconds == INT64_MAX` also makes this expression
perform signed-overflow undefined behavior. Please make the rounding helper
checked/status-returning, reject values that leave Doris's 0001..9999 domain,
and cover the upper-domain and integer-overflow boundaries.
--
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]
