github-actions[bot] commented on code in PR #67784:
URL: https://github.com/apache/doris/pull/67784#discussion_r3980877005
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/fileformat/ParquetFileFormatProperties.java:
##########
@@ -61,7 +61,7 @@ public class ParquetFileFormatProperties extends
FileFormatProperties {
private TParquetCompressionType parquetCompressionType =
TParquetCompressionType.SNAPPY;
private boolean parquetDisableDictionary = false;
private TParquetVersion parquetVersion = TParquetVersion.PARQUET_1_0;
- private boolean enableInt96Timestamps = true;
+ private boolean enableInt96Timestamps = false;
Review Comment:
[P1] Preserve DATETIMEV2 wall-clock semantics in INT64 output
With this default, normal OUTFILE/EXPORT writes now take the INT64 branch,
but `VParquetTransformer::_parse_schema` still creates `DATETIMEV2` as
`timestamp[unit, session-time-zone]`. The serde consequently converts the value
through the write session zone, and Arrow emits `isAdjustedToUTC=true`; Doris
readers then convert that instant through the read session zone. For example, a
DATETIMEV2 `2023-04-20 00:00:00` written in Asia/Shanghai is read as
`2023-04-19 09:00:00` in America/Los_Angeles instead of preserving its
timezone-free fields. The new INT64 test reads in the same session, so it
misses this. Please use a timezone-naive Arrow timestamp for DATETIMEV2 on the
default INT64 path (while keeping TIMESTAMPTZ aware) and cover a cross-zone
round trip.
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -1628,6 +1633,8 @@ Status
TableReader::create_file_reader(std::unique_ptr<FileReader>* reader) {
const bool enable_mapping_varbinary = _scan_params != nullptr &&
_scan_params->__isset.enable_mapping_varbinary &&
Review Comment:
[P1] Apply the versioned INT96 contract to the V1 fallback
This is the only place that consumes `parquet_timestamp_semantics_version`,
but `enable_file_scanner_v2=false` still routes the same current-FE plan to
legacy `FileScanner`. That reader receives only the session timezone, so it
ignores an explicit `hive.parquet.time-zone` and also cannot honor the new
empty/default wall-clock semantics; the same query therefore returns different
timestamps solely based on the scanner toggle. Please pass the tri-state
contract through V1 as well, or reject/force V2 for version-1 plans, and
exercise the cross-zone INT96 round trip with V2 disabled.
##########
be/src/core/data_type_serde/data_type_string_serde.cpp:
##########
@@ -464,6 +466,45 @@ Status
DataTypeStringSerDeBase<ColumnType>::write_column_to_arrow(
}
}
+template <typename ColumnType>
+Status DataTypeStringSerDeBase<ColumnType>::write_column_to_iceberg(
+ const std::shared_ptr<const IDataType>& type, const IColumn& column,
+ const NullMap* null_map, const std::shared_ptr<arrow::Field>& field,
+ arrow::ArrayBuilder* array_builder, int64_t start, int64_t end,
Review Comment:
[P1] Keep old-FE Iceberg fixed writes compatible
During a BE-first rolling upgrade, the old FE still maps Iceberg `fixed(N)`
to `CHAR(N)`. Those plans now reach this Iceberg-specific converter on the
upgraded BE and fail here, although the pre-patch generic Arrow path accepted
the same fixed-size-binary target and padded CHAR values to N bytes. Making new
FEs always plan VARBINARY only fixes the opposite version direction. Please
retain a compatibility binding for `CHAR(N) -> fixed(N)` (while keeping new
plans on VARBINARY) and cover an old-plan/new-BE writer case.
--
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]
