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


##########
paimon-core/src/main/java/org/apache/paimon/table/source/KeyValueTableRead.java:
##########
@@ -129,11 +141,97 @@ public InnerTableRead withTopN(TopN topN) {
 
     @Override
     public InnerTableRead withLimit(int limit) {
-        initialized().forEach(r -> r.withLimit(limit));
         this.limit = limit;
+        refreshReaderConfig();
         return this;
     }
 
+    @Override
+    public RecordReader<InternalRow> createReader(List<Split> splits) throws 
IOException {
+        if (limit == null || limit <= 0) {
+            return createConcatenatedReader(splits);
+        }
+
+        if (needsTableLevelLimit(splits)) {
+            return createReaderWithGlobalLimit(splits);
+        }
+        return createConcatenatedReader(splits);
+    }
+
+    private boolean needsTableLevelLimit(List<Split> splits) {
+        if (splits.size() > 1) {
+            return true;
+        }
+        if (!isMergeReadLimitActive(splits.get(0))) {

Review Comment:
   This path now assumes the limited read has at least one split. An empty 
table, a partition/bucket filter that prunes everything, or a scan filter that 
produces an empty plan can still call 
`table.newRead().withLimit(n).createReader(plan.splits())` with `splits` empty; 
then `needsTableLevelLimit` reaches `splits.get(0)` and throws 
`IndexOutOfBoundsException` instead of returning an empty reader. Please handle 
the empty-list case before checking the single-split merge-limit status, and 
add a regression for `withLimit` over an empty plan.



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

Reply via email to