Copilot commented on code in PR #6711:
URL: https://github.com/apache/hive/pull/6711#discussion_r3825115246


##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/IcebergAcidUtil.java:
##########
@@ -261,12 +244,23 @@ public static class VirtualColumnAwareIterator<T> 
implements CloseableIterator<T
     private final GenericRecord current;
     private final Configuration conf;
 
-    public VirtualColumnAwareIterator(
-        CloseableIterator<T> currentIterator, Schema expectedSchema, 
Configuration conf) {
+    private final int specId;
+    private final long partitionHash;
+    private final String filePath;
+
+    public VirtualColumnAwareIterator(CloseableIterator<T> currentIterator, 
Schema expectedSchema,
+        Configuration conf, FileScanTask task) {
       this.currentIterator = currentIterator;
-      this.current = GenericRecord.create(
-          new Schema(expectedSchema.columns().subList(4, 
expectedSchema.columns().size())));
+      this.current = GenericRecord.create(new Schema(
+          expectedSchema.columns().subList(FILE_READ_META_COLS.size(), 
expectedSchema.columns().size())));
       this.conf = conf;

Review Comment:
   VirtualColumnAwareIterator currently builds the output record as 
`expectedSchema.columns().subList(FILE_READ_META_COLS.size(), ...)`, which will 
include any row-lineage metadata columns appended by 
`MetadataColumns.schemaWithRowLineage(...)`. That would leak virtual/metadata 
fields into the row object and can shift the expected column positions for Hive 
consumers.



##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/VirtualColumn.java:
##########
@@ -74,7 +74,8 @@ public enum VirtualColumn {
       ImmutableSet.of(FILENAME.getName(), BLOCKOFFSET.getName(),
           RAWDATASIZE.getName(), GROUPINGID.getName(), ROWID.getName(), 
ROWISDELETED.getName(),
           PARTITION_SPEC_ID.getName(), PARTITION_HASH.getName(), 
FILE_PATH.getName(), ROW_POSITION.getName(),
-          PARTITION_PROJECTION.getName(), ROW_LINEAGE_ID.getName(), 
LAST_UPDATED_SEQUENCE_NUMBER.getName());
+          PARTITION_NAME.getName(), ROW_LINEAGE_ID.getName(),
+          LAST_UPDATED_SEQUENCE_NUMBER.getName());

Review Comment:
   Renaming/removing the PARTITION__PROJECTION virtual column looks like a 
backward-incompatible user-facing change (queries referencing 
PARTITION__PROJECTION will stop resolving). If the intention is to replace it 
with PARTITION__NAME, consider keeping PARTITION__PROJECTION as a compatibility 
alias that resolves to PARTITION_NAME.



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