Gabriel39 commented on code in PR #66247:
URL: https://github.com/apache/doris/pull/66247#discussion_r3679650528
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonScanParams.java:
##########
@@ -140,7 +144,11 @@ public static Table applyOptions(Table table, Map<String,
String> options) {
.filter(key -> !tableOptions.containsKey(key))
.forEach(key -> isolatedOptions.put(key, null));
}
- return table.copy(isolatedOptions);
+ Table effectiveTable = table.copy(isolatedOptions);
+ // Validate after every copy so relation options participate in the
documented
+ // relation > catalog > physical precedence before the effective value
is judged.
+
PaimonReaderOptions.validateEffectiveTableOptions(effectiveTable.options());
Review Comment:
Fixed in 8ea52152291. `PaimonSysExternalTable` now applies and validates the
resolved relation options against the source data table hidden by
`ReadonlyTable`; ScanNode repeats that hidden-handle guard at the final
boundary. Type inspection uses only the raw wrapper so a safe relation override
is not rejected before the parameters arrive. Added a real Paimon
`PartitionsTable` unit fixture plus partitioned P0 no-override and
safe-override queries.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -995,18 +996,27 @@ private Table getProcessedTable() throws UserException {
throw new UserException("Can not specify scan params and table
snapshot at same time.");
}
+ Table finalTable;
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()) {
+ finalTable = baseTable.copy(getIncrReadParams());
+ } else if (theScanParams != null && theScanParams.isOptions()) {
try {
- return source.getPaimonTable(theScanParams);
+ finalTable = source.getPaimonTable(theScanParams);
} catch (IllegalArgumentException e) {
throw new UserException(e.getMessage(), e);
}
+ } else {
+ finalTable = baseTable;
+ }
+ try {
+ // This is the last common boundary before planning and
serialization, including scans
Review Comment:
Fixed in 8ea52152291. `PaimonExternalTable.fetchRowCount()` now validates
the effective base handle immediately before
`newReadBuilder().newScan().plan()`. Added coverage through both
`ExternalRowCountCache` and manual `AnalysisManager` job construction; each
test uses a real file-store table whose planning hook proves that planning
never starts after rejection.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -995,18 +996,27 @@ private Table getProcessedTable() throws UserException {
throw new UserException("Can not specify scan params and table
snapshot at same time.");
}
+ Table finalTable;
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()) {
+ finalTable = baseTable.copy(getIncrReadParams());
+ } else if (theScanParams != null && theScanParams.isOptions()) {
try {
- return source.getPaimonTable(theScanParams);
+ finalTable = source.getPaimonTable(theScanParams);
} catch (IllegalArgumentException e) {
throw new UserException(e.getMessage(), e);
}
+ } else {
+ finalTable = baseTable;
+ }
+ try {
+ // This is the last common boundary before planning and
serialization, including scans
+ // with no relation copy and incremental/system-table paths that
bypass applyOptions.
Review Comment:
Fixed in 8ea52152291. Snapshot binding for relation `@options` now builds
its projection from the already merged table copy, and partition loading
validates then enumerates that effective handle with
`CatalogUtils.listPartitionsFromFileSystem` instead of reloading the raw
physical table through the catalog. Added real partitioned LocalFileIO fixtures
for unsafe rejection and safe-copy enumeration, plus the partitioned P0 fixture.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/source/PaimonScanNode.java:
##########
@@ -995,18 +996,27 @@ private Table getProcessedTable() throws UserException {
throw new UserException("Can not specify scan params and table
snapshot at same time.");
}
+ Table finalTable;
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()) {
+ finalTable = baseTable.copy(getIncrReadParams());
+ } else if (theScanParams != null && theScanParams.isOptions()) {
try {
- return source.getPaimonTable(theScanParams);
+ finalTable = source.getPaimonTable(theScanParams);
} catch (IllegalArgumentException e) {
throw new UserException(e.getMessage(), e);
}
+ } else {
+ finalTable = baseTable;
+ }
+ try {
+ // This is the last common boundary before planning and
serialization, including scans
+ // with no relation copy and incremental/system-table paths that
bypass applyOptions.
+
PaimonReaderOptions.validateEffectiveTableOptions(finalTable.options());
Review Comment:
Fixed in 8ea52152291. `PaimonReaderOptions.validateEffectiveTable` now
recursively validates `FallbackReadFileStoreTable.fallback()` after the
applicable `Table.copy`, so both independently planned children must be safe.
The tests construct real Paimon main/fallback file-store tables and cover both
hidden unsafe rejection and a safe relation override.
--
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]
