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



##########
File path: spark3/src/main/java/org/apache/iceberg/spark/Spark3Util.java
##########
@@ -269,6 +279,51 @@ public Transform unknown(int fieldId, String sourceName, 
int sourceId, String tr
     return transforms.toArray(new Transform[0]);
   }
 
+  public static Distribution toRequiredDistribution(PartitionSpec spec, 
SortOrder sortOrder, boolean inferFromSpec) {
+    if (sortOrder.isUnsorted()) {
+      if (inferFromSpec) {
+        SortOrder specOrder = Partitioning.sortOrderFor(spec);
+        return Distributions.ordered(convert(specOrder));
+      }
+
+      return Distributions.unspecified();
+    }
+
+    Schema schema = spec.schema();
+    Multimap<Integer, SortField> sortFieldIndex = 
Multimaps.index(sortOrder.fields(), SortField::sourceId);
+
+    // build a sort prefix of partition fields that are not already in the 
sort order
+    SortOrder.Builder builder = SortOrder.builderFor(schema);
+    for (PartitionField field : spec.fields()) {
+      Collection<SortField> sortFields = sortFieldIndex.get(field.sourceId());
+      boolean isSorted = sortFields.stream().anyMatch(sortField ->
+              field.transform().equals(sortField.transform()) ||
+                      
sortField.transform().satisfiesOrderOf(field.transform()));
+      if (!isSorted) {
+        String sourceName = schema.findColumnName(field.sourceId());
+        
builder.asc(org.apache.iceberg.expressions.Expressions.transform(sourceName, 
field.transform()));
+      }
+    }
+
+    // add the configured sort to the partition spec prefix sort
+    SortOrderVisitor.visit(sortOrder, new CopySortOrderFields(builder));
+
+    return Distributions.ordered(convert(builder.build()));
+  }
+
+  public static Distribution toRequiredDistribution(PartitionSpec spec) {
+    if (spec.isUnpartitioned()) {

Review comment:
       I think the cases are reversed. If the spec is _un_-partitioned then the 
distribution should be unspecified. Otherwise, it should be clustered by the 
transforms.




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

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