mxm commented on code in PR #15433:
URL: https://github.com/apache/iceberg/pull/15433#discussion_r2964644977


##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicRecord.java:
##########
@@ -34,11 +34,25 @@ public class DynamicRecord {
   private Schema schema;
   private RowData rowData;
   private PartitionSpec partitionSpec;
-  private DistributionMode distributionMode;
+  @Nullable private DistributionMode distributionMode;
   private int writeParallelism;
   private boolean upsertMode;
   @Nullable private Set<String> equalityFields;
 
+  /**
+   * Constructs a new DynamicRecord with {@code null} distributionMode, 
indicating forward (no
+   * shuffle) writes.
+   */
+  public DynamicRecord(
+      TableIdentifier tableIdentifier,
+      String branch,
+      Schema schema,
+      RowData rowData,
+      PartitionSpec partitionSpec,
+      int writeParallelism) {
+    this(tableIdentifier, branch, schema, rowData, partitionSpec, null, 
writeParallelism);

Review Comment:
   ```suggestion
     public DynamicRecord(
         TableIdentifier tableIdentifier,
         String branch,
         Schema schema,
         RowData rowData,
         PartitionSpec partitionSpec) {
       this(tableIdentifier, branch, schema, rowData, partitionSpec, null, -1);
   ```
   
   Write parallelism doesn't make sense with the direct write path.



##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicRecord.java:
##########
@@ -59,7 +74,7 @@ public DynamicRecord(
       Schema schema,
       RowData rowData,
       PartitionSpec partitionSpec,
-      DistributionMode distributionMode,
+      @Nullable DistributionMode distributionMode,
       int writeParallelism) {

Review Comment:
   ```suggestion
         DistributionMode distributionMode,
         int writeParallelism) {
   ```
   
   We can add the Nullable to the field itself, but this constructor is for 
when we actually have a DistributionMode. IMHO the nullability of 
DistributionMode should be an implementation detail.



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