Gabriel39 commented on code in PR #68481:
URL: https://github.com/apache/doris/pull/68481#discussion_r4092852936
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -2577,16 +2577,18 @@ private Split createIcebergSplit(FileScanTask
fileScanTask) throws UserException
}
split.setTableFormatType(TableFormatType.ICEBERG);
split.setTargetSplitSize(selectFeSplitSize(fileScanTask,
targetSplitSize));
- if (isPartitionedTable) {
- int specId = fileScanTask.file().specId();
+ // REPLACE or partition evolution can leave an unpartitioned table
with historical specs.
+ // Row-level deletes must retain each file's spec instead of
defaulting to historical spec 0.
+ int specId = dataFile.specId();
+ split.setPartitionSpecId(specId);
+ PartitionData partitionData = (PartitionData) dataFile.partition();
+ if (partitionData != null) {
PartitionSpec partitionSpec = icebergTable.specs().get(specId);
Preconditions.checkNotNull(partitionSpec, "Partition spec with
specId %s not found for table %s",
specId, icebergTable.name());
- PartitionData partitionData = (PartitionData)
fileScanTask.file().partition();
- if (partitionData != null) {
- split.setPartitionSpecId(specId);
- split.setPartitionDataJson(IcebergUtils.getPartitionDataJson(
- partitionData, partitionSpec,
sessionVariable.getTimeZone()));
+ split.setPartitionDataJson(IcebergUtils.getPartitionDataJson(
Review Comment:
Fixed in eccf50af77. BINARY/FIXED now use the Iceberg Base64 representation
and a matching decoder, including fixed-length validation. UUID and TIME
decoding is supported, and unsupported serialization no longer silently becomes
NULL. Added real evolved-spec scan/Thrift/delete-metadata tests for all four
types, including empty and NULL binary values, plus direct/read-only buffer
coverage. The original binary/fixed cases reproduced NULL partition corruption
before the fix. All 197 tests across the four affected Iceberg test classes and
FE Checkstyle pass. The external regression now includes historical
binary-partition DELETE; it passes syntax validation but still needs end-to-end
CI execution.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -2577,16 +2577,18 @@ private Split createIcebergSplit(FileScanTask
fileScanTask) throws UserException
}
split.setTableFormatType(TableFormatType.ICEBERG);
split.setTargetSplitSize(selectFeSplitSize(fileScanTask,
targetSplitSize));
- if (isPartitionedTable) {
- int specId = fileScanTask.file().specId();
+ // REPLACE or partition evolution can leave an unpartitioned table
with historical specs.
+ // Row-level deletes must retain each file's spec instead of
defaulting to historical spec 0.
+ int specId = dataFile.specId();
+ split.setPartitionSpecId(specId);
+ PartitionData partitionData = (PartitionData) dataFile.partition();
+ if (partitionData != null) {
PartitionSpec partitionSpec = icebergTable.specs().get(specId);
Preconditions.checkNotNull(partitionSpec, "Partition spec with
specId %s not found for table %s",
specId, icebergTable.name());
- PartitionData partitionData = (PartitionData)
fileScanTask.file().partition();
- if (partitionData != null) {
- split.setPartitionSpecId(specId);
- split.setPartitionDataJson(IcebergUtils.getPartitionDataJson(
- partitionData, partitionSpec,
sessionVariable.getTimeZone()));
+ split.setPartitionDataJson(IcebergUtils.getPartitionDataJson(
+ partitionData, partitionSpec,
sessionVariable.getTimeZone()));
Review Comment:
Fixed in eccf50af77 with Math.floorDiv/Math.floorMod. Added real
partition-evolution tests for both TIMESTAMP and TIMESTAMPTZ at -1 and -1000001
microseconds, checking scan serialization and exact partition reconstruction in
delete metadata. They reproduced the negative NanoOfSecond exception before the
fix and now pass. The external regression also covers DELETE/UPDATE over
retained pre-epoch TIMESTAMP_NTZ partitions; end-to-end execution remains
pending CI.
--
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]
