github-actions[bot] commented on code in PR #66247:
URL: https://github.com/apache/doris/pull/66247#discussion_r3688023410
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -316,20 +317,60 @@ Table resolveTable(PaimonTableHandle paimonHandle) {
Table resolveScanTable(PaimonTableHandle paimonHandle) {
Table table = resolveTable(paimonHandle);
Map<String, String> scanOptions = paimonHandle.getScanOptions();
+ Table finalTable = table;
if (scanOptions != null && !scanOptions.isEmpty()) {
if (PaimonScanParams.isOptionsPin(scanOptions)) {
// An @options pin owns the whole scan-startup state:
applyOptions strips the internal
// markers and nulls out the absent members of paimon's
inherited read-state family, so a
// scan.mode / tag persisted on the base table cannot leak
into this relation's read.
- return PaimonScanParams.applyOptions(table, scanOptions);
+ finalTable = PaimonScanParams.applyOptions(table, scanOptions);
+ } else {
+ // FIX-INCR-SCAN-RESET: for an @incr read, reapply legacy's
null reset of
+ // scan.snapshot-id/scan.mode here (the single Table.copy
chokepoint shared by both the
+ // native/JNI scan path and the JNI serialized-table path) so
a stale persisted pin on the
+ // base table cannot hijack incremental-between.
Non-incremental pins pass through unchanged.
+ finalTable =
table.copy(PaimonIncrementalScanParams.applyResetsIfIncremental(scanOptions));
+ }
+ }
+ finalTable = runtimeSafeTable(finalTable);
Review Comment:
[P1] Normalize fallback and deferred BE planners too
The live system-wrapper thread covers the discarded hidden FE copy, but this
helper also misses an ordinary fallback/delegate whose visible main branch is
unset and hidden branch has a catalog-valid above-CPU value; recursive
validation rejects it instead of capping it. Separately, `$partitions`/`$files`
re-plan manifests after deserialization, so a value safe on a 32-core FE can
still grow Paimon's global pool on an 8-core BE. Please extend that fix to
recursively normalize fallback/delegate children and reapply the local cap
before deferred BE planning, with hidden-branch and heterogeneous-capacity
tests.
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -575,12 +670,31 @@ static Optional<Long> parseDataSizeBytes(String value) {
private void initTable() {
Preconditions.checkState(params.containsKey("serialized_table"));
table = PaimonUtils.deserialize(params.get("serialized_table"));
+
validateSerializedReadBatchSize(table.options().get(CoreOptions.READ_BATCH_SIZE.key()));
Review Comment:
[P1] Validate hidden readers in the rolling-upgrade guard
This check only inspects the deserialized wrapper's visible `options()`. A
`FallbackReadFileStoreTable` can therefore carry a safe main table and a hidden
fallback with `read.batch-size=0`; Paimon's `newRead()` constructs both readers
and selects the fallback one for fallback splits, so an older FE can still send
a table that passes this new guard and then makes no progress in the BE reader.
Please recurse through every readable fallback/delegated child here as the
FE-side validator does, and cover a serialized fallback wrapper in the
compatibility test.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonCatalogOps.java:
##########
@@ -267,21 +268,24 @@ public List<String> listTables(String databaseName)
throws Catalog.DatabaseNotEx
* #65955: overlay the catalog-level {@code paimon.table-option.*}
defaults onto the loaded
* table, exactly where legacy {@code
PaimonExternalCatalog.getPaimonTable} did — this is the
* connector's only {@code Catalog.getTable} call, so branch,
time-travel and system tables all
- * inherit the defaults. Options the table sets itself win (see {@link
PaimonTableOptions#forCopy}).
+ * inherit the configured reader policy. Catalog options override
physical table values;
+ * relation-scoped options can still override them for one scan.
*/
@Override
public Table getTable(Identifier identifier) throws
Catalog.TableNotExistException {
Table table = catalog.getTable(identifier);
- if (tableOptions.isEmpty()) {
- return table;
- }
- Map<String, String> optionsForCopy =
PaimonTableOptions.forCopy(tableOptions, table.options());
+ Map<String, String> optionsForCopy =
PaimonTableOptions.forCopy(tableOptions);
+ // Relation options are applied after this cached handle is
returned. Defer final
+ // validation so a safe relation value can override an unsafe
physical value.
return optionsForCopy.isEmpty() ? table :
table.copy(optionsForCopy);
}
@Override
- public List<Partition> listPartitions(Identifier identifier) throws
Catalog.TableNotExistException {
- return catalog.listPartitions(identifier);
+ public List<Partition> listPartitions(Identifier identifier, Table
table)
+ throws Catalog.TableNotExistException {
+ // The supplied handle already contains catalog and relation
policy. Reloading by identifier
+ // would discard those copies before manifest enumeration reaches
the final scan guard.
+ return CatalogUtils.listPartitionsFromFileSystem(table);
Review Comment:
[P1] Preserve REST-owned partition listing
This exact-table fix unconditionally takes Paimon's filesystem fallback, but
`RESTCatalog.listPartitions` first returns the REST server's partition result
and scans manifests only when that endpoint reports `NotImplementedException`.
For a supported metastore-partitioned REST catalog, this now ignores the
catalog-authoritative partition set/visibility and performs a different full
manifest listing. Please retain the REST/catalog-native path and use this
supplied effective table for its filesystem fallback, with a REST fixture whose
server result differs from manifest enumeration.
--
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]
