JingsongLi commented on code in PR #8423:
URL: https://github.com/apache/paimon/pull/8423#discussion_r3510935892
##########
paimon-core/src/main/java/org/apache/paimon/table/ChainGroupReadTable.java:
##########
@@ -112,6 +139,28 @@ public FileStoreTable copyWithLatestSchema() {
wrapped.copyWithLatestSchema(),
other().copyWithLatestSchema());
}
+ /** Prepares options for a specific branch in chain table. */
+ private static Map<String, String> prepareBranchOptions(
+ Map<String, String> options, String branch, FileStoreTable
sourceTable) {
+ Map<String, String> result = new HashMap<>(options);
Review Comment:
This still loses branch-specific table options on the SQL lookup path.
`BaseDataTableSource.timeTravelDisabledTable()` builds a schema from the outer
chain table options and calls `table.copy(newSchema)` before
`LookupFileStoreTable.create()` validates the chain table. In
`copy(TableSchema)`, this helper starts from `newTableSchema.options()` and
only restores `branch` and `bucket`, so a delta branch altered to
`merge-engine=partial-update` or `aggregation` is copied back as the main table
merge engine. The new validation then sees `DEDUPLICATE` and the unsupported
delta branch is accepted. Please preserve branch-owned options from
`sourceTable.schema().options()` (then overlay only safe dynamic scan options),
and add a SQL lookup test for partial-update / aggregation delta branches.
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/lookup/LookupFileStoreTable.java:
##########
@@ -73,6 +90,11 @@ public InnerTableRead newRead() {
@Override
public StreamDataTableScan newStreamScan() {
+ if (CoreOptions.fromMap(wrapped.options()).isChainTable()) {
+ ChainGroupReadTable chainGroupReadTable =
+ (ChainGroupReadTable) ((FallbackReadFileStoreTable)
wrapped).other();
+ return new ChainTableStreamScan(chainGroupReadTable);
Review Comment:
For chain-table lookup this scan checkpoints the delta branch position, but
the existing full-cache refresh logic still compares `reader.nextSnapshotId()`
with `context.table.snapshotManager().latestSnapshotId()` / the outer table
latest snapshot. Those are different snapshot sequences for chain tables. With
`lookup.refresh.async` or `lookup.refresh.full-load-threshold`, the backlog
calculation can be wrong and may skip the intended sync/full refresh behavior.
Please either make the lookup refresh path use the delta branch snapshot
manager for chain tables, or explicitly disable those backlog/full-load
threshold checks for chain-table lookup.
--
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]
