github-actions[bot] commented on code in PR #68934:
URL: https://github.com/apache/doris/pull/68934#discussion_r4245058048
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -964,7 +968,7 @@ && hasTrustworthyLimitAccounting(table)
Optional<List<RawFile>> optRawFiles =
dataSplit.convertToRawFiles();
Optional<List<DeletionFile>> optDeletionFiles =
dataSplit.deletionFiles();
- boolean nativeEligible =
shouldUseNativeReader(paimonHandle.isForceJni(),
+ boolean nativeEligible =
shouldUseNativeReader(paimonHandle.isForceJni() || needsNanosecondReader,
Review Comment:
[P2] Preserve metadata columns when routing nanosecond scans. A normal
Paimon table advertises `__paimon_file_path` and `__paimon_row_index`, but
selecting either with a `TIMESTAMP(7-9)` field sets `needsNanosecondReader` and
makes `nativeEligible` false here. The JNI branch then calls
`validateMetadataColumnReader(true, false)` and rejects every raw-convertible
split, whereas its FileScannerV2 native path previously served the query. This
needs a reader path that preserves both nanoseconds and physical metadata, plus
a regression query covering the combination.
##########
be/src/format_v2/orc/orc_reader.cpp:
##########
@@ -947,6 +953,12 @@ Status OrcReader::init(RuntimeState* state) {
try {
_state->reader = ::orc::createReader(std::move(input_stream), options);
_state->root_type = &_state->reader->getType();
+ if (orc_serde_utils::has_nanosecond_timestamp(*_state->root_type)) {
+ // Use the same UTC wall-clock carrier as the writer; historical
local offsets
+ // differ between ORC and cctz and can push boundary values out of
range.
+ _state->timezone = "GMT";
Review Comment:
[P1] Keep the ORC negative-fraction correction when changing reader
timezone. For an external Iceberg ORC stripe written with `Asia/Shanghai`, a
valid `timestamp_ns` wall time `1970-01-01 00:00:00.500000000` enters ORC 1.9.0
as second `-28800`; its writer stores `-28799` for the fractional-second
encoding. This new file-wide GMT reader mode shifts that to `+1`, so ORC's
reader skips its correction (which runs only for negative adjusted seconds),
and Doris returns `00:00:01.500000000`. A plain timestamp in the same file
follows the same path. Keep a compatible writer-zone conversion or correct the
boundary before materialization, and cover a non-GMT external stripe crossing
zero.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergPartitionUtils.java:
##########
@@ -479,6 +491,13 @@ static Object parsePartitionValueFromString(String
valueStr, Type icebergType, Z
case DATE:
// Iceberg date is days since epoch (1970-01-01).
return (int) LocalDate.parse(valueStr,
DateTimeFormatter.ISO_LOCAL_DATE).toEpochDay();
+ case TIMESTAMP_NANO:
+ LocalDateTime timestampNs =
LocalDateTime.parse(valueStr.replace('T', ' '),
Review Comment:
[P2] Accept the write-side date-only form for static timestamp_ns
partitions. `INSERT OVERWRITE ... PARTITION(ts='2024-01-01')` can materialize a
valid midnight `TIMESTAMP_NS` row because its BE cast accepts the ten-character
date, but the static partition map retains the raw string and this new
`TIMESTAMP_NANO` parser requires `HH:mm:ss`. `buildPartitionFilter` then fails
during overwrite commit, after writing files. Normalize the static value to the
typed timestamp or parse the accepted date-only form here, and cover it with a
static identity overwrite test.
##########
be/src/format/transformer/vorc_transformer.cpp:
##########
@@ -349,6 +355,11 @@ std::unique_ptr<orc::Type>
VOrcTransformer::_build_orc_type(
type = orc::createPrimitiveType(orc::DATE);
break;
}
+ case TYPE_TIMESTAMP_NS: {
+ type = orc::createPrimitiveType(orc::TIMESTAMP);
+ type->setAttribute("iceberg.timestamp-unit", "NANOS");
Review Comment:
[P2] Preserve ORC nanoseconds in TVF and LOAD reads. This writer marks the
field `iceberg.timestamp-unit=NANOS`, but legacy
`OrcReader::convert_to_doris_type` ignores the attribute and infers
`DATETIMEV2(6)` for TVF schema fetch and LOAD source columns. With
`enable_file_scanner_v2=false`, a TVF scan uses that reader's `nanoseconds /
1000` conversion, returning `.123456789` as `.123456`; LOAD also uses the
legacy reader even when V2 is requested. Handle the annotation in inference and
decoding, or route both paths through a lossless reader, and test both round
trips.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonTypeMapping.java:
##########
@@ -156,6 +156,10 @@ private static ConnectorType toDecimalType(DecimalType
type) {
private static ConnectorType toTimestampType(DataType dataType) {
int scale = DEFAULT_TIMESTAMP_SCALE;
if (dataType instanceof TimestampType) {
+ // Keep all source fractional digits instead of truncating
high-precision wall-clock values.
+ if (((TimestampType) dataType).getPrecision() >
MAX_TIMESTAMP_SCALE) {
+ return ConnectorType.of("TIMESTAMP_NS");
Review Comment:
[P2] Preserve access to high-precision Paimon values outside the nanosecond
range. This maps every existing Paimon `TimestampType(7-9)` to `TIMESTAMP_NS`,
whose signed 64-bit epoch range is only about 1677-2262. A valid existing value
such as `1000-01-01 00:00:00` previously mapped to `DATETIMEV2(6)`, but the JNI
`appendTimestampNs` path now reaches `Math.multiplyExact` in
`TypeNativeBytes.convertToTimestampNs` and aborts the scan. Please retain a
compatibility route or make the narrower range opt-in for existing tables, and
test an out-of-range historical row.
##########
be/src/exec/sink/writer/iceberg/viceberg_partition_writer.cpp:
##########
@@ -84,10 +109,14 @@ Status VIcebergPartitionWriter::open(RuntimeState* state,
RuntimeProfile* profil
open_status = parquet_compression_type.error();
break;
}
- ParquetFileOptions parquet_options = {.compression_type =
parquet_compression_type.value(),
- .parquet_version =
TParquetVersion::PARQUET_1_0,
- .parquet_disable_dictionary =
false,
- .enable_int96_timestamps =
false};
+ // Parquet 1.0 coerces Arrow nanoseconds to microseconds, including
nested fields.
+ ParquetFileOptions parquet_options = {
+ .compression_type = parquet_compression_type.value(),
+ .parquet_version =
contains_nanosecond_timestamp(_schema.root_struct())
Review Comment:
[P2] Preserve Parquet nanoseconds in TVF and LOAD reads. This new Parquet 2
write path emits unadjusted `TIMESTAMP(NANOS)`, but legacy
`format/parquet/schema_desc.cpp` declares it `DATETIMEV2(6)`. TVF schema fetch
fixes that type before forced FileScannerV2 decodes the physical
`TIMESTAMP_NS`, so `ColumnMapper` rounds `.123456789` to `.123457`. LOAD
disables V2 and its legacy `Int64ToTimestamp` converter keeps only
microseconds. Update both schema inference and legacy load conversion (or route
them through lossless decoding), and test both file round trips.
--
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]
