github-actions[bot] commented on code in PR #68259:
URL: https://github.com/apache/doris/pull/68259#discussion_r4217192667


##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -510,18 +518,26 @@ private void initializePartitionMetadata() {
         }
         enableMappingVarbinaryForPartitionMetadata = 
getEnableMappingVarbinary();
         enableMappingTimestampTzForPartitionMetadata = 
getEnableMappingTimestampTz();
-        orderedPathPartitionKeys = Collections.unmodifiableList(
-                IcebergUtils.getCommonIdentityPartitionColumns(icebergTable,
-                        enableMappingVarbinaryForPartitionMetadata,
-                        enableMappingTimestampTzForPartitionMetadata));
         if (sessionVariable.enableFileScannerV2) {
-            orderedPartitionMetadataKeys = Collections.unmodifiableList(
+            // Classify every identity column of EVERY spec as a partition 
key, like master does. A file
+            // written under an older identity spec carries its value in the 
manifest only, and BE uses a
+            // split partition value just for a column FE marked as a 
partition key, so the intersection
+            // below would leave that value unused and the column read as 
NULL. Files that carry no value
+            // for the column (a spec without it) are unaffected: the v2 
column mapper falls back to the
+            // physical field when the split has no partition value for a 
partition key.
+            orderedPathPartitionKeys = Collections.unmodifiableList(

Review Comment:
   [P2] Preserve NULL fallback when a historical partition key is fetched 
lazily. For a table with an old unpartitioned file written before optional `p` 
was added, then an `identity(p)` spec, this all-spec classification marks `p` 
as `PARTITION_KEY` even on the old file, which has neither a split value nor a 
physical `p`. A TopN query that fetches `p` in phase two copies that category 
via `column_name_to_category`, but 
`RowIdStorageReader::build_external_scan_params()` supplies an empty default 
expression for the lazy slot. `TableColumnMapper` then returns `does not have a 
matching partition value` instead of the NULL that an ordinary read produces. 
Preserve the optional-field default in phase-two params or let a missing 
optional partition-key value take the normal NULL path, and cover this 
mixed-spec lazy read.



-- 
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]

Reply via email to