JingsongLi commented on code in PR #8116:
URL: https://github.com/apache/paimon/pull/8116#discussion_r3458234493
##########
paimon-core/src/main/java/org/apache/paimon/table/source/KeyValueTableRead.java:
##########
@@ -129,11 +141,91 @@ 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()) {
Review Comment:
This check runs before the split reader is normally initialized. In the
common table-read path
(`table.newRead().withLimit(n).createReader(plan.splits())`), none of the
`SplitReadProvider` lazy fields has been materialized yet, so `initialized()`
is empty and `isMergeReadLimitActive()` returns false. That sends even a safe
single-split merge read through `createReaderWithGlobalLimit`, which disables
`applyMergeReadLimit` and means the new merge-read limit optimization does not
actually run for the normal production path unless the same `TableRead`
happened to create a reader earlier. Could we initialize/check the provider
that matches the single split before deciding this fallback, and add a
regression that verifies the safe single-split table-read path really applies
the merge-read limit instead of only the outer `LimitRecordReader`?
--
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]
