Re: [PR] [Core] Support to order data by columns in append only writer [paimon]

["JingsongLi (via GitHub)" <gi...@apache.org>]

JingsongLi commented on code in PR #7886:
URL: https://github.com/apache/paimon/pull/7886#discussion_r3458861961


##########
paimon-core/src/main/java/org/apache/paimon/append/AppendOnlyWriter.java:
##########
@@ -159,11 +164,18 @@ public AppendOnlyWriter(
         this.statsCollectorFactories = statsCollectorFactories;
         this.maxDiskSize = maxDiskSize;
         this.fileIndexOptions = fileIndexOptions;
+        this.coreOptions = coreOptions;
 
-        this.sinkWriter =
-                useWriteBuffer
-                        ? createBufferedSinkWriter(spillable)
-                        : new DirectSinkWriter<>(this::createRollingRowWriter);
+        // Determine if we need to enable sorting based on clustering configuration
+        List<String> clusteringColumns = coreOptions.clusteringColumns();
+        this.sortEnabled =
+                coreOptions.clusteringIncrementalEnabled()
+                        && coreOptions.clusteringIncrementalOptimizeWrite()
+                        && coreOptions.clusteringIncrementalMode()
+                                == CoreOptions.ClusteringIncrementalMode.LOCAL_SORT
+                        && !clusteringColumns.isEmpty();
+
+        this.sinkWriter = createSinkWriter(useWriteBuffer, spillable);

Review Comment:
   This makes local sort silently depend on `write-buffer-for-append`, whose default is `false`. Even when all clustering-local-sort options above make `sortEnabled=true`, the default append path passes `useWriteBuffer=false`, so `createSinkWriter` falls through to `DirectSinkWriter` and no sorting happens. The new test sets `write-buffer-for-append=true`, but the incremental clustering configuration/docs do not require users to set that extra option, so the feature will appear enabled while being bypassed in the common path. Could local-sort force the buffered writer here (or fail fast/document and test the required `write-buffer-for-append=true` setting)?



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org