github-actions[bot] commented on code in PR #66247:
URL: https://github.com/apache/doris/pull/66247#discussion_r3697996362
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonJniScanner.java:
##########
@@ -523,64 +628,156 @@ static Optional<Long> parseDataSizeBytes(String value) {
if (value == null || value.trim().isEmpty()) {
return Optional.empty();
}
- String normalized = value.trim().toLowerCase(Locale.ROOT).replace("_",
"").replace(" ", "");
- int unitStart = 0;
- while (unitStart < normalized.length()
- && (Character.isDigit(normalized.charAt(unitStart)) ||
normalized.charAt(unitStart) == '.')) {
- unitStart++;
- }
- if (unitStart == 0) {
- return Optional.empty();
- }
try {
- double number = Double.parseDouble(normalized.substring(0,
unitStart));
- String unit = normalized.substring(unitStart);
- long multiplier;
- switch (unit) {
- case "":
- case "b":
- case "byte":
- case "bytes":
- multiplier = 1L;
- break;
- case "k":
- case "kb":
- case "kib":
- multiplier = 1024L;
- break;
- case "m":
- case "mb":
- case "mib":
- multiplier = 1024L * 1024L;
- break;
- case "g":
- case "gb":
- case "gib":
- multiplier = 1024L * 1024L * 1024L;
- break;
- case "t":
- case "tb":
- case "tib":
- multiplier = 1024L * 1024L * 1024L * 1024L;
- break;
- default:
- return Optional.empty();
- }
- return Optional.of((long) (number * multiplier));
- } catch (NumberFormatException e) {
+ // Keep the BE guard's accepted grammar identical to the Paimon
option parser that will
+ // consume this value; accepting a superset lets invalid
serialized options reach scans.
+ return Optional.of(MemorySize.parse(value).getBytes());
+ } catch (IllegalArgumentException e) {
return Optional.empty();
}
}
private void initTable() {
Preconditions.checkState(params.containsKey("serialized_table"));
table = PaimonUtils.deserialize(params.get("serialized_table"));
+ String encodedSystemSource = params.get(PAIMON_OPTION_PREFIX +
DORIS_SERIALIZED_SYSTEM_SOURCE);
+ FileStoreTable systemSource = encodedSystemSource == null
+ ? null : PaimonUtils.deserialize(encodedSystemSource);
+ table = applyBackendManifestParallelism(table,
+ params.get(PAIMON_OPTION_PREFIX +
DORIS_MANIFEST_PARALLELISM_CAP),
+ Runtime.getRuntime().availableProcessors(), systemSource,
+ params.get(PAIMON_OPTION_PREFIX + DORIS_SYSTEM_TABLE_TYPE));
+ validateSerializedReaderOptions(table);
Review Comment:
[P1] Backstop fallback options hidden by legacy system wrappers
With an older FE, the auxiliary system source is absent. For a reader-backed
view such as `$audit_log`, `$binlog`, or `$row_tracking` over a
`FallbackReadFileStoreTable`, the outer wrapper exposes only the pair's main
options, so this validates the safe main branch and cannot descend to the
fallback child. `newRead()` then delegates through the wrapper and constructs
both readers, allowing a fallback-only `read.batch-size=0` or out-of-range
async threshold to reach the reader path. Please add a legacy system-wrapper
backstop for fallback-only reader options and a real serialized wrapper test.
The live split-target thread covers a different option.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindRelation.java:
##########
@@ -636,6 +637,18 @@ private Optional<LogicalPlan> handleMetaTable(TableIf
table, UnboundRelation unb
if (sysTablePlan.isNative()) {
List<String> qualifierWithoutTableName =
qualifiedTableName.subList(0, qualifiedTableName.size() - 1);
ExternalTable sysExternalTable =
sysTablePlan.getSysExternalTable();
+ if (sysExternalTable instanceof PluginDrivenSysExternalTable) {
+ Optional<TableSnapshot> tableSnapshot =
unboundRelation.getTableSnapshot();
+ Optional<TableScanParams> scanParams =
Optional.ofNullable(unboundRelation.getScanParams());
+ StatementContext statementContext =
cascadesContext.getStatementContext();
+ ((PluginDrivenSysExternalTable)
sysExternalTable).resolveScanPin(
Review Comment:
[P2] Use the system relation's pin for row-count statistics
This stores the source snapshot only in the transient system table's private
memo. `PluginDrivenSysExternalTable` is intentionally not an `MvccTable`, so
its inherited `getRowCount()` cannot retrieve this entry from
`StatementContext`; it falls through to the latest-keyed cache and Paimon's
two-argument statistics path, while schema and scan initialization consume the
memoized pin. Thus `$ro@options('scan.snapshot-id'='S')` can execute S but be
costed at S+1 (and an empty fence can be costed after the first commit). Please
thread this same memoized connector snapshot into the three-argument statistics
call, or return UNKNOWN rather than reopen latest, and cover positive plus
empty system pins.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExecuteCommand.java:
##########
@@ -95,22 +96,25 @@ public void run(ConnectContext ctx, StmtExecutor executor)
throws Exception {
// per-statement scope so a prior execution's cached tables/state
never leak into this one (the
// scope key's queryId is a second line of defense). See
StatementContext#resetConnectorStatementScope.
statementContext.resetConnectorStatementScope();
+ statementContext.resetMvccSnapshots();
LogicalPlan logicalPlan = prepareCommand.getLogicalPlan();
- LogicalPlan relationRoot = logicalPlan;
+ List<LogicalPlan> relationRoots = new ArrayList<>();
if (logicalPlan instanceof InsertIntoTableCommand) {
- relationRoot = ((InsertIntoTableCommand)
logicalPlan).getLogicalQuery();
+ relationRoots.add(((InsertIntoTableCommand)
logicalPlan).getLogicalQuery());
} else if (logicalPlan instanceof InsertOverwriteTableCommand) {
- relationRoot = ((InsertOverwriteTableCommand)
logicalPlan).getLogicalQuery();
+ relationRoots.add(((InsertOverwriteTableCommand)
logicalPlan).getLogicalQuery());
} else if (logicalPlan instanceof UpdateCommand) {
- relationRoot = ((UpdateCommand) logicalPlan).getLogicalQuery();
- } else if (logicalPlan instanceof Command) {
- // Non-DML commands deliberately have no traversable children;
they cannot own a
- // relation scan tree whose resolved state needs resetting.
- relationRoot = null;
+ relationRoots.add(((UpdateCommand) logicalPlan).getLogicalQuery());
+ } else if (logicalPlan instanceof DeleteFromUsingCommand) {
Review Comment:
[P1] Reset retained OPTIONS state in prepared base DELETE subqueries
This arm covers only `DeleteFromUsingCommand`. The parser still builds the
base `DeleteFromCommand` for DELETE without USING/CTE/order-limit, and that
command retains its `logicalQuery`; all other Commands are skipped here. Its
external relations occur in predicate subqueries, whose
`SubqueryExpr.queryPlan` also is not reached by ordinary Plan-child traversal.
Consequently a prepared `DELETE ... WHERE id IN (SELECT id FROM
source@options('scan.mode'='latest'))` can keep the first EXECUTE's
`resolvedMapParams` after a commit and mutate against the stale snapshot even
though the StatementContext maps were cleared. Reset the base DELETE root and
recursively visit retained subquery plans, then add a repeated-EXECUTE
regression.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonScanPlanProvider.java:
##########
@@ -1226,6 +1288,12 @@ private Table resolveSchemaDictTable(Table table,
PaimonTableHandle handle) {
return table;
}
if (table instanceof ReadOptimizedTable) {
+ FileStoreTable pinnedSource = handle.getSysBaseTable();
Review Comment:
[P1] Build the `$ro` dictionary from the option-applied source
`withScanOptions` deliberately leaves `sysBaseTable` unchanged, while
`resolveScanTable` applies an explicit historical OPTIONS selector to the `$ro`
wrapper/source. Returning the handle's original latest source here pairs the
pinned scan with a different schema dictionary. After `old` is renamed to
`new`, `$ro@options('scan.snapshot-id'='S')` binds `old`, but both schemas
consulted by `resolveCurrentSchemaFields` come from the latest source and the
scan fails that `old` is absent (or can attach later field/type metadata to
pinned files). Retain/recover the exact effective source used by the final
wrapper for dictionary construction and test a historical `$ro` across
rename/type evolution.
--
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]
