github-actions[bot] commented on code in PR #68481:
URL: https://github.com/apache/doris/pull/68481#discussion_r4129079613
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -2589,16 +2589,29 @@ 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) {
Review Comment:
[P2] Avoid serializing partition JSON for ordinary unpartitioned reads. For
files with Iceberg's empty `PartitionData`, this branch now looks up the spec
and emits `[]` for every split even when the scan does not project
`__DORIS_ICEBERG_ROWID_COL__`. That adds list/Gson allocations and a Thrift
payload per split in large ordinary scans; the BE readers consume this JSON
only while producing that hidden row ID. Please compute the row-ID requirement
once and generate the JSON only for scans that need it, while retaining the
per-file spec ID and the strict rejection for unsupported row-ID partitions.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -2589,16 +2589,29 @@ 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);
+ try {
split.setPartitionDataJson(IcebergUtils.getPartitionDataJson(
partitionData, partitionSpec,
sessionVariable.getTimeZone()));
+ } catch (UnsupportedOperationException e) {
+ // Dropped source columns leave UNKNOWN types in historical
specs. Reads do not need
+ // this row-ID payload, but DML must never replace an
unrepresentable partition with NULL.
+ boolean requiresRowId = desc.getSlots().stream().anyMatch(slot
-> slot.getColumn() != null
+ &&
Column.ICEBERG_ROWID_COL.equalsIgnoreCase(slot.getColumn().getName()));
+ if (requiresRowId) {
+ throw new UserException("Cannot produce Iceberg row IDs
with unsupported partition types in spec "
Review Comment:
[P2] Preserve this DML error through the split wrapper. DELETE, UPDATE, and
MERGE disable external-table batch mode during planning, so `FileQueryScanNode`
calls `getSplits`. That method catches this `UserException` and wraps
`ExceptionUtils.getRootCauseMessage(e)`. Its root cause is the serializer's
`UnsupportedOperationException`, so the client receives only the UNKNOWN-type
error and loses this message, spec ID, and rewrite guidance; the new `exception
"Cannot produce Iceberg row IDs..."` assertions cannot match. Preserve the
`UserException` message when wrapping or rethrow it, and assert the surfaced
error in a planner-level test.
--
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]
