github-actions[bot] commented on code in PR #65984:
URL: https://github.com/apache/doris/pull/65984#discussion_r3649192715
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -924,8 +930,14 @@ private Table getProcessedTable() throws UserException {
}
if (theScanParams != null && theScanParams.incrementalRead()) {
+ // System table handles are cached, so preserve query isolation by
applying dynamic
+ // options to a copied Paimon table instead of changing the shared
handle.
return baseTable.copy(getIncrReadParams());
}
+ if (theScanParams != null && theScanParams.isOptions()) {
+ // Apply per-query options to a copy so they cannot leak through
cached table handles.
+ return baseTable.copy(theScanParams.getMapParams());
Review Comment:
[P1] Avoid inheriting the cached snapshot selector
An ordinary `OPTIONS` relation is loaded through the latest-snapshot branch
first, so `baseTable` already contains `scan.snapshot-id=<latest>`. Paimon's
`copy` merges this map instead of replacing it; a valid query such as
`t@options('scan.tag-name'='tag1')` therefore retains both `scan.snapshot-id`
and `scan.tag-name`, and Paimon rejects the scan because only one time-travel
selector may be set. Snapshot-id tests miss this because that key overwrites
the inherited value. Please resolve OPTIONS at the MVCC load/binding boundary,
or explicitly remove inherited mutually exclusive selectors before copying, and
cover tag/timestamp selection.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/TableScanParams.java:
##########
@@ -29,12 +29,14 @@ public class TableScanParams {
public static final String INCREMENTAL_READ = "incr";
public static final String BRANCH = "branch";
public static final String TAG = "tag";
+ public static final String OPTIONS = "options";
public static final String SNAPSHOT = "snapshot";
public static final String RESET = "reset";
private static final ImmutableSet<String> VALID_PARAM_TYPES =
ImmutableSet.of(
INCREMENTAL_READ,
BRANCH,
- TAG);
+ TAG,
+ OPTIONS);
Review Comment:
[P2] Validate the OPTIONS shape and target
This makes `OPTIONS` globally valid, but there is no mandatory capability or
argument check afterward. The shared grammar accepts `@options()` and
`@options(foo)`; Paimon reads only `mapParams`, so both become silent default
scans. The same object is propagated to Hive/Iceberg and other file scans that
never consume it, so an unsupported request such as
`hive_t@options('scan.snapshot-id'='1')` also succeeds against normal data.
Please require a non-empty map for OPTIONS and reject it for table types that
do not implement it, with negative analyzer coverage for all three cases.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
