github-actions[bot] commented on code in PR #68441:
URL: https://github.com/apache/doris/pull/68441#discussion_r4089252078
##########
gensrc/thrift/DataSinks.thrift:
##########
@@ -384,6 +384,9 @@ struct THiveTableSink {
10: optional bool overwrite
11: optional THiveSerDeProperties serde_properties
12: optional list<Types.TNetworkAddress> broker_addresses;
+ // Field 13 is supports_deferred_azure_multipart (bool) in master; never
reuse its wire ID.
+ // Absent: legacy session timezone; empty: wall-clock INT96; otherwise:
named catalog timezone.
+ 14: optional string hive_parquet_time_zone
Review Comment:
[P1] Keep Hive timestamp writes stable across the 4.1-to-4.2 upgrade
This FE now always sends field 14, and patched 4.1 BEs use it to select
wall-clock/catalog-zone INT96 normalization. The supported next branch
(`branch-4.2`) still ends `THiveTableSink` at field 12 and its Hive writer uses
the session timezone, so it skips this unknown field. Doris's documented
BE-first, stepwise minor upgrade therefore creates a mixed-BE window where one
distributed Hive INSERT can write different INT96 carriers whenever the session
and catalog/default zones differ. Please forward-port this field and writer
handling to 4.2 before releasing this change, or capability-gate the new
normalization so every backend participating in one insert uses the same
contract.
##########
be/src/exec/operator/file_scan_operator.cpp:
##########
@@ -147,9 +148,26 @@ bool FileScanLocalState::should_use_file_scanner_v2(const
TQueryOptions& query_o
const bool is_transactional_hive =
scan_params.__isset.table_format_params &&
scan_params.table_format_params.table_format_type ==
"transactional_hive";
- return query_options.__isset.enable_file_scanner_v2 &&
query_options.enable_file_scanner_v2 &&
- !is_load && scan_params.format_type !=
TFileFormatType::FORMAT_ES_HTTP &&
- !is_transactional_hive;
+ // Hybrid scans advertise native Parquet at scan level because ranges can
arrive after the
+ // scanner is selected; FORMAT_JNI alone does not imply that every range
uses JNI.
+ const bool is_hybrid_native_parquet = scan_params.format_type ==
TFileFormatType::FORMAT_JNI &&
+
scan_params.__isset.contains_native_parquet &&
+ scan_params.contains_native_parquet;
+ // Version 1 introduces the explicit wall-clock/instant contract that
scanner V1 cannot honor.
+ const bool requires_parquet_timestamp_contract =
+ (scan_params.format_type == TFileFormatType::FORMAT_PARQUET ||
+ is_hybrid_native_parquet ||
supports_iceberg_scan_semantics_v1(&scan_params)) &&
+ (scan_params.__isset.hive_parquet_time_zone ||
+ (scan_params.__isset.parquet_timestamp_semantics_version &&
+ scan_params.parquet_timestamp_semantics_version >= 1));
+ const bool scanner_v2_requested =
(query_options.__isset.enable_file_scanner_v2 &&
+ query_options.enable_file_scanner_v2) ||
+ requires_parquet_timestamp_contract;
+ // Iceberg's default write format does not describe retained files, and
remote splits arrive
+ // after scanner construction. Keep versioned Iceberg scans on V2 without
eagerly listing
+ // every file.
+ return scanner_v2_requested && !is_load &&
Review Comment:
[P1] Keep insert-only transactional Parquet on the versioned reader path
The final `!is_transactional_hive` veto overrides
`requires_parquet_timestamp_contract` for every transactional Hive table. Full
ACID is ORC-only, but `HMSExternalTable.isFullAcidTable()` rejects Parquet only
for full ACID, so insert-only transactional Parquet remains a supported path.
Those scans are marked transactional at scan level, select scanner V1 here, and
later use the ordinary V1 Hive Parquet wrapper for their non-ACID ranges. V1
receives only the session timezone and never reads
`hive_parquet_time_zone`/`parquet_timestamp_semantics_version`. Consequently
the version-1 default (empty = preserve INT96 wall clock) still shifts values
by the session zone, and a configured catalog zone is ignored. Please let
supported insert-only transactional Parquet use V2 (while retaining the
ORC/full-ACID exception), or teach its V1 path the same three-state contract.
##########
be/src/format_v2/parquet/reader/native_column_reader.cpp:
##########
@@ -202,6 +202,37 @@ const NativeFieldSchema* find_child_field(const
NativeFieldSchema& parent,
return field_it == parent.children.end() ? nullptr : &*field_it;
}
+} // namespace
+
+Status detail::sync_native_field_types(const ParquetColumnSchema& schema,
+ NativeFieldSchema* field) {
+ DORIS_CHECK(field != nullptr);
+ if (schema.kind == ParquetColumnSchemaKind::VARIANT) {
+ DORIS_CHECK(schema.variant_physical_type != nullptr);
+ // Native dispatch still needs Variant's physical group after
request-level logical types
+ // are applied; replacing it with TYPE_VARIANT would lose every
physical leaf index.
+ field->data_type = schema.variant_physical_type;
+ field->variant_physical_type = schema.variant_physical_type;
+ } else {
+ field->data_type = schema.type;
Review Comment:
[P1] Carry the request timestamp semantic into native decoding
`apply_projection_timestamp_semantics()` records the authoritative semantic
in `ParquetColumnSchema`, and statistics use it, but this synchronization
copies only `data_type`. `init_decode_context()` later reparses
`isAdjustedToUTC` from the untouched footer. This mismatch is reachable without
corrupt metadata: Spark can write a Hive `timestamp` as INT64
`TIMESTAMP(adjusted=true)`, while Paimon 1.4.2's Hive migration renames that
file without rewriting it and maps Hive timestamp to Paimon's no-zone
timestamp, so Doris receives marker `false` for the matching history schema.
Rows are then shifted using footer `true` while statistics/pruning use request
`false`. Please propagate the effective semantic recursively into the
reader-owned decode context (or pass it directly), and cover migrated annotated
INT64 rows plus predicate pruning in a non-UTC session.
--
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]
