JingsongLi commented on code in PR #10860:
URL: https://github.com/apache/paimon/pull/10860#discussion_r4451560687
##########
paimon-core/src/main/java/org/apache/paimon/table/FallbackReadFileStoreTable.java:
##########
@@ -246,7 +276,57 @@ public DataTableScan
newFallbackScan(Function<FileStoreTable, DataTableScan> sca
validateSchema();
FileStoreTable first = wrappedFirst ? wrapped : other;
FileStoreTable second = wrappedFirst ? other : wrapped;
- return new FallbackReadScan(first, second, wrapped.schema(),
scanCreator);
+ if (coreOptions().isChainTable()) {
+ return new FallbackReadScan(first, second, wrapped.schema(),
scanCreator);
+ }
+ boolean otherIsEmpty =
+ Objects.equals(other.coreOptions().scanSnapshotId(),
EMPTY_BRANCH_SNAPSHOT_ID)
+ || (other.coreOptions().scanSnapshotId() != null
+ && other.snapshotManager().latestSnapshotId()
== null);
Review Comment:
[P1] Resolve the empty branch for factory-constructed reads too
The mutable emptiness fallback is still used by
`FileStoreTableFactory.create(...)`: that path constructs both physical tables
with the same scan selector and never calls `rewriteOtherOptions`, so
`other.scanSnapshotId()` remains 1 rather than the new -1 sentinel. I extended
this PR's `testTimeTravelWithEmptyOtherBranch` with only
`assertThat(readAndCollect(fromFactory, scan -> {})).containsExactly(Pair.of(1,
10))` immediately after its existing first branch write. All four cases fail,
returning `[1=10, 2=20]`; the extra row was committed strictly after the
requested main snapshot, including with `allow-earliest-snapshot=false`. The
fixture already proves the same factory table returns only main before that
write, but only checks the copy()-created wrapper afterward. Please run the
same timestamp/empty resolution when creating a fallback/primary wrapper
through the public factory API and retain that decision, rather than rechecking
mutable branch state here. This finding concerns
that SDK/factory path; the repaired copy() path passes.
--
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]
