github-actions[bot] commented on code in PR #66297:
URL: https://github.com/apache/doris/pull/66297#discussion_r3698463998
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonSysExternalTable.java:
##########
@@ -135,19 +153,69 @@ public Table getSysPaimonTable() {
}
public Table getSysPaimonTable(TableScanParams scanParams) {
- Table table = getSysPaimonTable();
if (scanParams == null || !scanParams.isOptions()) {
- return table;
+ return getSysPaimonTable();
}
- Map<String, String> resolvedOptions = scanParams.getOrResolveMapParams(
- options ->
PaimonScanParams.resolveOptions(sourceTable.getBasePaimonTable(), options));
+ Map<String, String> resolvedOptions = resolvedOptions(scanParams);
if
(PaimonScanParams.getPinnedFileCreationTime(resolvedOptions).isPresent()) {
// Generic system-table wrappers cannot carry Paimon's
manifest-entry predicate.
// Reject the fallback instead of silently widening it to the
whole pinned snapshot.
throw new IllegalArgumentException(
"Paimon system tables cannot apply a creation-time file
filter.");
}
- return PaimonScanParams.applyOptions(table, resolvedOptions);
+ FileStoreTable effectiveDataTable = (FileStoreTable)
PaimonScanParams.applyOptions(
Review Comment:
[P1] Build OPTIONS system tables from the bound fence generation.
BindRelation stores the base table's exact relationSnapshot, but PaimonSource
ignores it for PaimonSysExternalTable and this path instead applies the
fence-injected snapshot selector to the wrapper's cached raw source. If
invalidation/schema ALTER separates those handles, the normal copy can rewind
to the snapshot's old schema instead of retaining the fence's latest-schema
table generation, making system-table binding/rows alias-order dependent. This
is the system-table counterpart left after the ordinary-relation fence fix.
Thread the fenced PaimonSnapshot.table through schema and scan construction and
use the no-time-travel projection semantics.
##########
fe/fe-core/src/test/java/org/apache/doris/tablefunction/PaimonTableValuedFunctionTest.java:
##########
@@ -0,0 +1,56 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.tablefunction;
+
+import org.apache.doris.datasource.paimon.PaimonReaderOptions;
+
+import org.apache.paimon.CoreOptions;
+import org.apache.paimon.table.FileStoreTable;
+import org.apache.paimon.table.Table;
+import org.apache.paimon.table.system.PartitionsTable;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import java.util.Collections;
+import java.util.Map;
+
+public class PaimonTableValuedFunctionTest {
+
+ @Test
+ void testMetadataTableWrapsCpuCappedDataTable() {
+ int localCapacity = Runtime.getRuntime().availableProcessors();
+ Assumptions.assumeTrue(localCapacity <
PaimonReaderOptions.MAX_MANIFEST_PARALLELISM);
+ FileStoreTable dataTable = Mockito.mock(FileStoreTable.class);
+ FileStoreTable safeDataTable = Mockito.mock(FileStoreTable.class);
+ Mockito.when(dataTable.options()).thenReturn(Collections.singletonMap(
+ CoreOptions.SCAN_MANIFEST_PARALLELISM.key(),
String.valueOf(localCapacity + 1)));
+
Mockito.when(safeDataTable.options()).thenReturn(Collections.singletonMap(
+ CoreOptions.SCAN_MANIFEST_PARALLELISM.key(),
String.valueOf(localCapacity)));
+
Mockito.when(dataTable.copy(Mockito.anyMap())).thenReturn(safeDataTable);
Review Comment:
[P1] Stub the copy method this test actually drives. The assumption plus
configured localCapacity + 1 forces runtimeSafeTable into
normalizeManifestParallelism's over-cap branch, which calls
copyWithoutTimeTravel(...), not copy(...). Mockito therefore returns null here
and createRuntimeSafeSystemTable fails in validateEffectiveTable before
reaching these assertions on every host where the test runs; the final verify
also targets the wrong method. Stub/verify copyWithoutTimeTravel with the
expected cap map, as the other normalization tests do.
##########
fe/fe-core/src/main/java/org/apache/doris/tablefunction/PaimonTableValuedFunction.java:
##########
@@ -98,12 +101,27 @@ public PaimonTableValuedFunction(TableName
paimonTableName, String queryType) th
));
NameMapping buildNameMapping = externalTable.getOrBuildNameMapping();
- this.paimonSysTable =
paimonExternalCatalog.getPaimonTable(buildNameMapping,
- "main", queryType);
+ this.paimonSysTable = createRuntimeSafeSystemTable(
+ paimonExternalCatalog.getPaimonTable(buildNameMapping),
queryType);
this.schema = PaimonUtil.parseSchema(paimonSysTable,
paimonExternalCatalog.getEnableMappingVarbinary(),
paimonExternalCatalog.getEnableMappingTimestampTz());
}
+ static Table createRuntimeSafeSystemTable(Table dataTable, String
queryType) {
+ if (!(dataTable instanceof FileStoreTable)) {
+ throw new IllegalArgumentException("Paimon metadata queries
require a file-store data table.");
+ }
+ // System-table wrappers hide the data table that owns manifest
planning. Normalize the
+ // disposable data handle before wrapping it so metadata TVFs cannot
bypass the CPU cap.
+ FileStoreTable safeDataTable = (FileStoreTable)
PaimonReaderOptions.runtimeSafeTable(dataTable);
+ PaimonReaderOptions.validateEffectiveTable(safeDataTable);
+ Table systemTable = SystemTableLoader.load(queryType, safeDataTable);
Review Comment:
[P1] Keep the fallback pair as the direct source of the rebuilt $ro table.
When this handle is PrivilegedFileStoreTable(FallbackReadFileStoreTable(...))
and its manifest values already fit the local bound, runtimeSafeTable returns
that wrapper unchanged. Paimon 1.3.1 ReadOptimizedTable.newScan() only builds
FallbackReadScan when its immediate wrapped table is
FallbackReadFileStoreTable; behind the privilege delegate it takes the
single-table path and can omit fallback-only rows. The prior catalog
system-table lookup did not introduce this ordering, and PaimonSysExternalTable
already uses PaimonTableDecorators.unwrapToFallbackOrBase. Apply the same
invariant before SystemTableLoader.load and cover a privilege-around-fallback
query_type='ro'.
--
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]
