Gabriel39 commented on code in PR #66297:
URL: https://github.com/apache/doris/pull/66297#discussion_r3690587611
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -248,7 +263,11 @@ public BaseAnalysisTask createAnalysisTask(AnalysisInfo
info) {
public long fetchRowCount() {
makeSureInitialized();
long rowCount = 0;
- List<Split> splits =
getBasePaimonTable().newReadBuilder().newScan().plan().splits();
+ Table effectiveTable = getBasePaimonTable();
+ // Statistics and row-count cache planning run before ScanNode and
must not reach an
+ // unsafe manifest executor, even when the foreground relation later
supplies an override.
+ PaimonReaderOptions.validateEffectiveTable(effectiveTable);
Review Comment:
Valid. Manifest-planning consumers now build runtime-capped execution
copies, including hidden system sources.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java:
##########
@@ -958,8 +961,61 @@ public Optional<MvccSnapshot> getSnapshot(TableIf tableIf)
{
if (!(tableIf instanceof MvccTable)) {
return Optional.empty();
}
- MvccTableInfo mvccTableInfo = new MvccTableInfo(tableIf);
- return Optional.ofNullable(snapshots.get(mvccTableInfo));
+ MvccTableInfo defaultKey = new MvccTableInfo(tableIf);
+ MvccSnapshot defaultSnapshot = snapshots.get(defaultKey);
+ if (defaultSnapshot != null) {
+ return Optional.of(defaultSnapshot);
+ }
+ MvccSnapshot only = null;
+ for (Map.Entry<MvccTableInfo, MvccSnapshot> entry :
snapshots.entrySet()) {
+ if (defaultKey.isSameTable(entry.getKey())) {
+ if (only != null) {
+ return Optional.empty();
Review Comment:
Valid. Table-only descriptor consumers use a safe pinned projection when
multiple relation projections exist.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java:
##########
@@ -967,8 +980,74 @@ public Optional<MvccSnapshot> getSnapshot(TableIf tableIf)
{
if (!(tableIf instanceof MvccTable)) {
return Optional.empty();
}
- MvccTableInfo mvccTableInfo = new MvccTableInfo(tableIf);
- return Optional.ofNullable(snapshots.get(mvccTableInfo));
+ MvccTableInfo defaultKey = new MvccTableInfo(tableIf);
+ MvccSnapshot defaultSnapshot = snapshots.get(defaultKey);
+ if (defaultSnapshot != null) {
+ return Optional.of(defaultSnapshot);
+ }
+ MvccSnapshot only = null;
+ for (Map.Entry<MvccTableInfo, MvccSnapshot> entry :
snapshots.entrySet()) {
+ if (defaultKey.isSameTable(entry.getKey())) {
+ if (only != null) {
+ return Optional.empty();
+ }
+ only = entry.getValue();
+ }
+ }
+ return Optional.ofNullable(only);
+ }
+
+ public Optional<MvccSnapshot> getSnapshot(TableIf tableIf,
Optional<TableSnapshot> tableSnapshot,
+ Optional<TableScanParams> scanParams) {
+ if (!(tableIf instanceof MvccTable)) {
+ return Optional.empty();
+ }
+ return Optional.ofNullable(snapshots.get(
+ new MvccTableInfo(tableIf, versionKeyOf(tableSnapshot,
scanParams))));
+ }
+
+ /**
+ * Return a validated statement projection for metadata consumers without
relation identity.
+ */
+ public Optional<MvccSnapshot> getSnapshotForTableMetadata(TableIf tableIf)
{
+ Optional<MvccSnapshot> unambiguous = getSnapshot(tableIf);
+ if (unambiguous.isPresent() || !(tableIf instanceof MvccTable)) {
+ return unambiguous;
+ }
+ // Descriptor serialization has no relation key. Reuse a validated
statement projection
+ // instead of reopening a neutral handle after multiple OPTIONS
aliases were bound.
+ return Optional.ofNullable(tableMetadataSnapshots.get(new
MvccTableInfo(tableIf)));
+ }
+
+ private static String versionKeyOf(Optional<TableSnapshot> tableSnapshot,
+ Optional<TableScanParams> scanParams) {
+ // Limit the backport to relation-scoped OPTIONS: branch-4.1's older
Iceberg/Paimon
+ // time-travel paths still resolve their handles outside this map,
while OPTIONS needs an
+ // exact content key shared by analysis and scan planning.
+ if (scanParams != null && scanParams.isPresent() &&
scanParams.get().isOptions()) {
+ TableScanParams params = scanParams.get();
+ StringBuilder key = new StringBuilder("p");
+ appendVersionKeyPart(key, params.getParamType());
+ Map<String, String> sortedParams = new
TreeMap<>(params.getMapParams());
Review Comment:
Valid. Latest snapshot identity is shared independently from
relation-specific planning projections.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/paimon/PaimonLatestSnapshotProjectionLoader.java:
##########
@@ -69,6 +71,44 @@ public PaimonSnapshotCacheValue load(NameMapping
nameMapping, Table paimonTable)
}
}
+ public PaimonSnapshotCacheValue loadFence(NameMapping nameMapping, Table
paimonTable) {
+ try {
+ // A statement fence needs version/schema identity only;
enumerating partitions here
+ // can fail before relation-level options have replaced an unsafe
physical setting.
+ return new PaimonSnapshotCacheValue(
+ PaimonPartitionInfo.EMPTY,
resolveLatestSnapshot(paimonTable));
+ } catch (Exception e) {
+ throw new CacheException("failed to load paimon snapshot fence
%s.%s.%s: %s",
+ e, nameMapping.getCtlId(), nameMapping.getLocalDbName(),
nameMapping.getLocalTblName(),
+ e.getMessage());
+ }
+ }
+
+ public PaimonSnapshotCacheValue loadAtFence(
+ NameMapping nameMapping, Table paimonTable, PaimonSnapshot fence) {
+ try {
+ FileStoreTable latestSchemaTable = ((FileStoreTable)
paimonTable).copyWithLatestSchema();
Review Comment:
Valid. Commit d6d387df114 carries the exact fenced table generation into
later aliases instead of reopening current metadata.
##########
fe/be-java-extensions/paimon-connector/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -571,17 +670,137 @@ static Optional<Long> parseDataSizeBytes(String value) {
private void initTable() {
Preconditions.checkState(params.containsKey("serialized_table"));
table = PaimonUtils.deserialize(params.get("serialized_table"));
+ table = applyBackendManifestParallelism(table,
+ params.get(PAIMON_OPTION_PREFIX +
DORIS_MANIFEST_PARALLELISM_CAP),
+ Runtime.getRuntime().availableProcessors());
+ table = applyDefaultReadBatchSize(table, batchSize);
+ paimonAllFieldNames = PaimonUtils.getFieldNames(this.table.rowType());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("paimonAllFieldNames:{}", paimonAllFieldNames);
+ }
+ }
+
+ static Table applyDefaultReadBatchSize(Table table, int dorisBatchSize) {
+ validateSerializedReaderOptions(table);
+ if (hasReadBatchSize(table)) {
+ // Doris' output block size and Paimon's reader batch are
independent controls; an
+ // explicitly validated value on any hidden reader must survive
transport unchanged.
+ return table;
+ }
// The serialized table may pin an older data snapshot while carrying
the latest schema
// after a schema change. Applying a normal copy would time travel to
that snapshot's
// schema again and make renamed or newly added columns disappear.
Map<String, String> readOptions = Collections.singletonMap(
- CoreOptions.READ_BATCH_SIZE.key(), String.valueOf(batchSize));
- table = table instanceof FileStoreTable
+ CoreOptions.READ_BATCH_SIZE.key(),
String.valueOf(dorisBatchSize));
+ return table instanceof FileStoreTable
? ((FileStoreTable) table).copyWithoutTimeTravel(readOptions)
: table.copy(readOptions);
- paimonAllFieldNames = PaimonUtils.getFieldNames(this.table.rowType());
- if (LOG.isDebugEnabled()) {
- LOG.debug("paimonAllFieldNames:{}", paimonAllFieldNames);
+ }
+
+ static Table applyBackendManifestParallelism(
+ Table table, String feParallelismCap, int localCapacity) {
+ List<Integer> configuredValues = new ArrayList<>();
+ collectManifestParallelism(table, configuredValues);
+ int requestedBound = localCapacity;
+ if (feParallelismCap != null) {
+ requestedBound =
Math.min(parsePositiveManifestParallelism(feParallelismCap), localCapacity);
+ }
+ final int safeBound = requestedBound;
+ // The FE cap is a requested bound, not proof that every serialized
wrapper carries it;
+ // a later table rebuild can expose the original physical value to
this BE.
+ if (configuredValues.isEmpty()
Review Comment:
Valid. Commit d6d387df114 transports the exact hidden system source and
rebuilds a real PartitionsTable under the smaller BE cap.
##########
fe/be-java-extensions/paimon-connector/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -571,17 +670,137 @@ static Optional<Long> parseDataSizeBytes(String value) {
private void initTable() {
Preconditions.checkState(params.containsKey("serialized_table"));
table = PaimonUtils.deserialize(params.get("serialized_table"));
+ table = applyBackendManifestParallelism(table,
+ params.get(PAIMON_OPTION_PREFIX +
DORIS_MANIFEST_PARALLELISM_CAP),
+ Runtime.getRuntime().availableProcessors());
+ table = applyDefaultReadBatchSize(table, batchSize);
+ paimonAllFieldNames = PaimonUtils.getFieldNames(this.table.rowType());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("paimonAllFieldNames:{}", paimonAllFieldNames);
+ }
+ }
+
+ static Table applyDefaultReadBatchSize(Table table, int dorisBatchSize) {
+ validateSerializedReaderOptions(table);
+ if (hasReadBatchSize(table)) {
+ // Doris' output block size and Paimon's reader batch are
independent controls; an
+ // explicitly validated value on any hidden reader must survive
transport unchanged.
+ return table;
+ }
// The serialized table may pin an older data snapshot while carrying
the latest schema
// after a schema change. Applying a normal copy would time travel to
that snapshot's
// schema again and make renamed or newly added columns disappear.
Map<String, String> readOptions = Collections.singletonMap(
- CoreOptions.READ_BATCH_SIZE.key(), String.valueOf(batchSize));
- table = table instanceof FileStoreTable
+ CoreOptions.READ_BATCH_SIZE.key(),
String.valueOf(dorisBatchSize));
+ return table instanceof FileStoreTable
? ((FileStoreTable) table).copyWithoutTimeTravel(readOptions)
: table.copy(readOptions);
- paimonAllFieldNames = PaimonUtils.getFieldNames(this.table.rowType());
- if (LOG.isDebugEnabled()) {
- LOG.debug("paimonAllFieldNames:{}", paimonAllFieldNames);
+ }
+
+ static Table applyBackendManifestParallelism(
+ Table table, String feParallelismCap, int localCapacity) {
+ List<Integer> configuredValues = new ArrayList<>();
+ collectManifestParallelism(table, configuredValues);
+ int requestedBound = localCapacity;
+ if (feParallelismCap != null) {
+ requestedBound =
Math.min(parsePositiveManifestParallelism(feParallelismCap), localCapacity);
+ }
+ final int safeBound = requestedBound;
+ // The FE cap is a requested bound, not proof that every serialized
wrapper carries it;
+ // a later table rebuild can expose the original physical value to
this BE.
+ if (configuredValues.isEmpty()
+ || configuredValues.stream().noneMatch(value -> value >
safeBound)) {
+ return table;
+ }
+ int safeParallelism = Math.min(
+
configuredValues.stream().mapToInt(Integer::intValue).min().getAsInt(),
+ safeBound);
+ Map<String, String> cap = Collections.singletonMap(
+ CoreOptions.SCAN_MANIFEST_PARALLELISM.key(),
String.valueOf(safeParallelism));
+ // Preserve the FE-selected schema while lowering only the BE-local
execution bound.
+ return table instanceof FileStoreTable
+ ? ((FileStoreTable) table).copyWithoutTimeTravel(cap)
+ : table.copy(cap);
+ }
+
+ private static int parsePositiveManifestParallelism(String value) {
+ try {
+ int parsed = Integer.parseInt(value);
+ if (parsed < 1) {
+ throw new IllegalArgumentException("Paimon manifest
parallelism cap must be positive.");
+ }
+ return parsed;
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("Paimon manifest parallelism
cap must be an integer.", e);
+ }
+ }
+
+ private static void collectManifestParallelism(Table table, List<Integer>
values) {
+ String configured =
table.options().get(CoreOptions.SCAN_MANIFEST_PARALLELISM.key());
+ if (configured != null) {
+ values.add(parsePositiveManifestParallelism(configured));
+ }
+ if (table instanceof FallbackReadFileStoreTable) {
+ collectManifestParallelism(((FallbackReadFileStoreTable)
table).fallback(), values);
+ }
+ if (table instanceof DelegatedFileStoreTable) {
+ collectManifestParallelism(((DelegatedFileStoreTable)
table).wrapped(), values);
+ }
+ }
+
+ private static boolean hasReadBatchSize(Table table) {
+ if (table.options().containsKey(CoreOptions.READ_BATCH_SIZE.key())) {
+ return true;
+ }
+ if (table instanceof FallbackReadFileStoreTable
+ && hasReadBatchSize(((FallbackReadFileStoreTable)
table).fallback())) {
+ return true;
+ }
+ return table instanceof DelegatedFileStoreTable
+ && hasReadBatchSize(((DelegatedFileStoreTable)
table).wrapped());
+ }
+
+ private static void validateSerializedReaderOptions(Table table) {
+
validateSerializedReadBatchSize(table.options().get(CoreOptions.READ_BATCH_SIZE.key()));
+
validateSerializedAsyncThreshold(table.options().get(CoreOptions.FILE_READER_ASYNC_THRESHOLD.key()));
+ if (table instanceof FallbackReadFileStoreTable) {
+ validateSerializedReaderOptions(((FallbackReadFileStoreTable)
table).fallback());
+ }
+ if (table instanceof DelegatedFileStoreTable) {
+ validateSerializedReaderOptions(((DelegatedFileStoreTable)
table).wrapped());
+ }
+ }
+
+ private static void validateSerializedAsyncThreshold(String value) {
+ if (value == null) {
+ return;
+ }
+ Optional<Long> bytes = parseDataSizeBytes(value);
Review Comment:
Valid. Commit d6d387df114 uses Paimon MemorySize parsing and covers long IEC
units plus boundaries.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -191,6 +193,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)) {
Review Comment:
Valid. Commit d6d387df114 classifies reader-only tuning from raw user
options before injecting the statement snapshot selector.
--
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]