Gabriel39 commented on code in PR #66247:
URL: https://github.com/apache/doris/pull/66247#discussion_r3690582100
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -178,6 +178,19 @@ private PaimonSnapshotCacheValue
getPaimonSnapshotCacheValue(Optional<TableSnaps
"Failed to get Paimon snapshot: " + (e.getMessage() ==
null ? "unknown cause" : e.getMessage()),
e);
}
+ } else if (scanParams.isPresent() && scanParams.get().isOptions()) {
+ Table baseTable = getBasePaimonTable();
+ Map<String, String> resolvedOptions =
scanParams.get().getOrResolveMapParams(
+ options -> PaimonScanParams.resolveOptions(baseTable,
options));
+ Table effectiveTable = PaimonScanParams.applyOptions(baseTable,
resolvedOptions);
+ if (PaimonScanParams.hasOnlyReaderOptions(resolvedOptions)) {
+ // Reader tuning cannot change snapshot metadata. Reuse the
memoized projection so
+ // a per-query batch-size change does not enumerate every
partition again.
+ return PaimonUtils.getLatestSnapshotCacheValue(this);
+ }
+ // The shared latest cache was built from the catalog-scoped
handle. Relation options
+ // need their own projection so partition enumeration uses the
final safe table copy.
+ return PaimonUtils.loadSnapshotProjection(this, effectiveTable);
Review Comment:
Valid. The current head keeps option-bearing projections relation-scoped and
covers both alias orders.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/paimon/PaimonPartitionInfoLoader.java:
##########
@@ -46,7 +45,10 @@ public PaimonPartitionInfo load(NameMapping nameMapping,
Table paimonTable, List
return PaimonPartitionInfo.EMPTY;
}
try {
- List<Partition> paimonPartitions =
tableLoader.catalog(nameMapping).getPaimonPartitions(nameMapping);
+ // Catalog.listPartitions reloads the raw physical table and loses
Doris catalog/relation
+ // copies. Enumerate the already merged handle so validation and
planning see one table.
+ PaimonReaderOptions.validateEffectivePlanningTable(paimonTable);
Review Comment:
Valid. The current head avoids neutral pre-lock preload for relation-scoped
options and covers the mixed-table path.
##########
fe/fe-connector/fe-connector-spi/src/main/java/org/apache/doris/connector/spi/ConnectorProvider.java:
##########
@@ -105,6 +106,18 @@ default void validateProperties(Map<String, String>
properties) {
// no-op by default
}
+ /**
+ * Validates an ALTER CATALOG candidate without publishing it to the live
catalog.
+ * Connectors with legacy-property compatibility rules may override this
method.
+ */
+ default void validatePropertiesForUpdate(
Review Comment:
Valid. The connector SPI major and compatibility gate expectations were
updated with the surface change.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -311,20 +312,45 @@ 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));
+ }
+ }
+ // This is the last common boundary before planning and serialization.
Validate only after
+ // relation and incremental copies establish relation > catalog >
physical precedence.
+ PaimonReaderOptions.validateEffectiveTable(finalTable);
+ validateHiddenSystemDataTable(paimonHandle, scanOptions);
+ return finalTable;
+ }
+
+ private void validateHiddenSystemDataTable(PaimonTableHandle handle,
Map<String, String> scanOptions) {
+ if (!handle.isSystemTable()) {
+ return;
+ }
+ try {
+ Table dataTable = catalogOps.getTable(
Review Comment:
Valid. The exact system-table source handle is retained and reused for scan
and statistics validation.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -465,6 +467,12 @@ public Optional<ConnectorTableHandle>
getSysTableHandle(ConnectorSession session
Table sysTable;
try {
sysTable = context.executeAuthenticated(() -> {
+ Table source = base.getPaimonTable();
Review Comment:
Valid. The read-optimized schema dictionary now uses the exact pinned source
generation.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java:
##########
@@ -1276,21 +1289,26 @@ private List<ConnectorPartitionInfo>
collectPartitions(PaimonTableHandle paimonH
return Collections.emptyList();
}
- // Partition enumeration is intentionally BASE-only: branch /
time-travel reads carry EMPTY
- // partition info (legacy PaimonPartitionInfo.EMPTY) and never reach
this path, so for the
- // (non-branch) handles that do, resolveTable returns the base table
and the base-Identifier
- // listing below is consistent. (A branch handle would otherwise mix
branch schema metadata
- // here with the base partition list — but that combination does not
occur by design.)
- Table table = resolveTable(paimonHandle);
+ Table resolvedTable = resolveTable(paimonHandle);
+ boolean optionsPin =
PaimonScanParams.isOptionsPin(paimonHandle.getScanOptions());
+ Table table;
+ if (optionsPin) {
+ table = PaimonScanParams.applyOptions(resolvedTable,
paimonHandle.getScanOptions());
+ } else {
+ // Partition projection never opens a data reader, so reader-only
settings must not
+ // invalidate metadata that a later relation-scoped override can
make safe.
+ PaimonReaderOptions.validateEffectivePlanningTable(resolvedTable);
+ table = resolvedTable;
+ }
Identifier identifier = Identifier.create(
paimonHandle.getDatabaseName(), paimonHandle.getTableName());
- // M-11: wrap the remote listPartitions in executeAuthenticated
(D-052), mirroring legacy
- // PaimonExternalCatalog.getPaimonPartitions which ran it inside
executionAuthenticator.execute
- // and swallowed TableNotExistException INSIDE the wrap (Kerberos
UGI.doAs would otherwise wrap
- // the checked exception, so it must be caught inside).
List<Partition> paimonPartitions;
try {
paimonPartitions = context.executeAuthenticated(() -> {
+ if (optionsPin) {
Review Comment:
Valid. Partition enumeration now preserves the resolved catalog reader
policy while retaining the native REST path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java:
##########
@@ -1131,7 +1131,7 @@ private static String
versionKeyOf(Optional<TableSnapshot> tableSnapshot,
}
if (scanParams != null && scanParams.isPresent()) {
TableScanParams sp = scanParams.get();
-
key.append("p:").append(sp.getParamType()).append(':').append(sp.getMapParams())
+ key.append("p:").append(sp.getParamType()).append(':').append(new
TreeMap<>(sp.getMapParams()))
Review Comment:
Valid. Snapshot selector keys now use a structurally injective encoding with
collision and order coverage.
##########
fe/fe-connector/pom.xml:
##########
@@ -55,7 +55,7 @@ under the License.
of the latter two means bumping this property as well (and
fe-extension-spi means bumping
all four families).
-->
- <connector.plugin.api.version>1.0</connector.plugin.api.version>
+ <connector.plugin.api.version>2.0</connector.plugin.api.version>
Review Comment:
Valid. Classpath providers are now subject to the same fail-closed plugin
API major gate.
##########
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);
+ // This is the last common boundary before planning and serialization.
Normalize and
+ // validate only after relation > catalog > physical precedence is
established.
+ PaimonReaderOptions.validateEffectiveTable(finalTable);
+ validateHiddenSystemDataTable(paimonHandle, scanOptions);
+ return finalTable;
+ }
+
+ private Table runtimeSafeTable(Table table) {
+ Map<String, String> runtimeOptions =
PaimonReaderOptions.runtimeSafeCopyOptions(
+ table, Collections.emptyMap());
+ // The cached catalog handle remains hardware-neutral; only the
query-local planning copy
+ // receives a CPU-local cap before it can resize Paimon's JVM-wide
manifest executor.
+ return runtimeOptions.isEmpty() ? table : table.copy(runtimeOptions);
+ }
+
+ private void validateHiddenSystemDataTable(PaimonTableHandle handle,
Map<String, String> scanOptions) {
+ if (!handle.isSystemTable()) {
+ return;
+ }
+ try {
+ Table dataTable = handle.getSystemTableSource();
+ if (dataTable == null) {
+ dataTable = handle.getSysBaseTable();
+ }
+ if (dataTable == null) {
+ dataTable = catalogOps.getTable(
+ Identifier.create(handle.getDatabaseName(),
handle.getTableName()));
}
- // 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.
- return
table.copy(PaimonIncrementalScanParams.applyResetsIfIncremental(scanOptions));
+ if (PaimonScanParams.isOptionsPin(scanOptions)) {
+ // Read-only system wrappers plan manifests through their
hidden data table, so the
+ // same relation copy must establish precedence on both
visible and hidden handles.
+ PaimonScanParams.applyOptions(dataTable, scanOptions);
Review Comment:
Valid. System wrappers are rebuilt from the normalized hidden source instead
of discarding the capped copy.
--
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]
