JingsongLi commented on code in PR #8116:
URL: https://github.com/apache/paimon/pull/8116#discussion_r3446043033
##########
paimon-core/src/main/java/org/apache/paimon/table/source/KeyValueTableRead.java:
##########
@@ -129,11 +140,58 @@ 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 (splits.size() > 1) {
+ return createReaderWithGlobalLimit(splits);
+ }
+
+ if (!isMergeReadLimitActive()) {
+ return LimitRecordReader.limit(createConcatenatedReader(splits),
limit);
+ }
+ return createConcatenatedReader(splits);
+ }
+
+ private RecordReader<InternalRow> createReaderWithGlobalLimit(List<Split>
splits)
+ throws IOException {
+ boolean previousApplyMergeReadLimit = applyMergeReadLimit;
+ applyMergeReadLimit = false;
+ refreshReaderConfig();
+ try {
+ return LimitRecordReader.limit(createConcatenatedReader(splits),
limit);
Review Comment:
`createConcatenatedReader(splits)` is lazy when there is more than one
split: it only stores suppliers that call `createReader(split)` later. This
method restores `applyMergeReadLimit` in the `finally` block before any of
those suppliers are consumed, so the later per-split readers are created with
`withLimit(limit)` enabled again, despite this multi-split path trying to
disable it. The outer `LimitRecordReader` still caps the final count, but rows
can be dropped before late filters that are applied outside
`MergeFileSplitRead` (for example row-level auth filters from `QueryAuthSplit`
in `AbstractDataTableRead.authedReader`): each split may be truncated before
the auth filter runs, and the global wrapper cannot recover the filtered-out
rows. Please keep merge-read limit disabled until the lazy readers are actually
consumed (or materialize the split readers while disabled), and add a
multi-split regression test with a late/applied-after-read filter.
--
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]
