rdblue commented on a change in pull request #4047:
URL: https://github.com/apache/iceberg/pull/4047#discussion_r805431271



##########
File path: 
spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/TestSparkDistributionAndOrderingUtil.java
##########
@@ -1567,6 +1567,370 @@ public void 
testRangePositionDeltaUpdatePartitionedTable() {
         table, UPDATE, expectedDistribution, 
SPEC_ID_PARTITION_FILE_POSITION_ORDERING);
   }
 
+  // 
==================================================================================
+  // Distribution and ordering for merge-on-read MERGE operations with 
position deletes
+  // 
==================================================================================
+  //
+  // UNPARTITIONED UNORDERED
+  // -------------------------------------------------------------------------
+  // merge mode is NOT SET -> rely on write distribution and ordering as a 
basis
+  // merge mode is NONE -> unspecified distribution + LOCALLY ORDER BY 
_spec_id, _partition, _file, _pos
+  // merge mode is HASH -> unspecified distribution + LOCALLY ORDER BY 
_spec_id, _partition, _file, _pos
+  // merge mode is RANGE -> unspecified distribution + LOCALLY ORDER BY 
_spec_id, _partition, _file, _pos
+  //
+  // UNPARTITIONED ORDERED BY id, data
+  // -------------------------------------------------------------------------
+  // merge mode is NOT SET -> rely on write distribution and ordering as a 
basis
+  // merge mode is NONE -> unspecified distribution +
+  //                       LOCALLY ORDER BY _spec_id, _partition, _file, _pos, 
id, data
+  // merge mode is HASH -> unspecified distribution +

Review comment:
       Why is the distribution unspecified when mode is hash here? Shouldn't 
this hash distribute by original data file (or maybe original partition), then 
by the new partition? Or is the fear that this will create too many small tasks 
by dividing original data files by the new partitioning?
   
   If that's the case, then it seems like this is optimizing for the case where 
you're running a MERGE with data from an old partition spec. I'd rather 
optimize for the case where the partition spec matches.




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