JingsongLi commented on code in PR #9966:
URL: https://github.com/apache/paimon/pull/9966#discussion_r4104996802
##########
paimon-core/src/main/java/org/apache/paimon/operation/DataEvolutionFileStoreScan.java:
##########
@@ -290,6 +318,35 @@ static List<ManifestEntry> pruneByReadType(
: kept;
}
+ /**
+ * Whether the row-id ranges of {@code entries} together cover every
{@code target} range
+ * (inclusive), with no gap. Dedicated files are always within the group's
full range, so
+ * covering the requested targets means the reader can derive the range
from them without the
+ * anchor.
+ */
+ private static boolean coversRanges(List<ManifestEntry> entries,
List<Range> targets) {
Review Comment:
[P1] This coverage check unions files from different projected columns. With
a normal file covering [0,9], b1.blob covering [0,6], and b2.blob covering
[3,9], the union covers [0,9], so the anchor is omitted. DataEvolutionSplitRead
still creates a separate BlobFileBunch per field, and SELECT b1, b2 fails its
first-row-ID alignment check (0 vs 3). I reproduced this with an end-to-end
test on this commit. Please retain the anchor unless each field bunch is
independently aligned with the requested rows, or keep it whenever pruning
leaves only dedicated files.
##########
paimon-core/src/main/java/org/apache/paimon/operation/DataEvolutionFileStoreScan.java:
##########
@@ -281,6 +286,29 @@ static List<ManifestEntry> pruneByReadType(
if (anchor != null && !kept.contains(anchor)) {
kept.add(anchor);
}
+ // Blob and vector-store files may each cover only a sub-range of
their group. If the kept
+ // files are all dedicated files that together do not cover the
requested rows (the whole
+ // group when there is no row-range pushdown), the reader would derive
the range from those
+ // sub-ranges and drop the rows outside them, so keep the full-range
anchor. When they
+ // already cover the requested rows, the anchor is unnecessary and
would only read an extra
+ // file.
+ if (anchor == null
+ && !kept.isEmpty()
+ && kept.stream()
+ .allMatch(
+ e ->
+ isBlobFile(e.file().fileName())
+ ||
isVectorStoreFile(e.file().fileName()))) {
+ ManifestEntry fullRangeAnchor = retrieveAnchorFile(group,
ManifestEntry::file);
+ Range fullRange = fullRangeAnchor.file().nonNullRowIdRange();
+ List<Range> requested =
+ rowRangeIndex == null
+ ? Collections.singletonList(fullRange)
+ : rowRangeIndex.intersectedRanges(fullRange.from,
fullRange.to);
+ if (!coversRanges(kept, requested)) {
+ kept.add(fullRangeAnchor);
Review Comment:
[P1] Retaining the full-range anchor here does not yet make a partially
populated vector column readable. A normal file covering [0,9] and a single
vector file covering [0,4], projected as SELECT embedding, produce field
bunches of 10 and 5 rows. DataEvolutionVectorReadPlanner returns null for a
single vector group, so DataEvolutionSplitRead.createUnionReader throws 'All
files in a field merge split should have the same row count.' I reproduced this
with an end-to-end test on this commit. Please align/null-fill the vector bunch
against the anchor's logical range (or use a range-aware reader), and add a
regression test without deletion vectors.
--
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]
