morningman commented on code in PR #69499:
URL: https://github.com/apache/doris/pull/69499#discussion_r4363772763
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergWriteSchemaContext.java:
##########
@@ -96,11 +94,12 @@ static IcebergWriteSchemaContext create(Table table, String
tableName,
Objects.requireNonNull(table, "table should not be null");
Objects.requireNonNull(tableName, "tableName should not be null");
Objects.requireNonNull(branchName, "branchName should not be null");
- Schema schema = branchName.isPresent()
- ? resolveBranchSchema(table, branchName.get(), tableName) :
table.schema();
if (branchName.isPresent()) {
- validateBranchWriterSchema(schema, table.schema(),
branchName.get(), tableName);
+ validateTargetBranch(table, branchName.get(), tableName);
}
+ // A branch selects only the parent snapshot / ref. Iceberg validates
a branch write against the
+ // table-current schema and stamps that schema on the new snapshot,
and Spark binds it the same way.
+ Schema schema = table.schema();
Review Comment:
Fixed in 49990114c95. The interleaving is real, and it is not specific to
branch writes: main-branch INSERT OVERWRITE on master has had the same exposure
since #65851 (the outer pass pins the writer schema and then expands DEFAULT
and bare rows by position against it) and #66529 (every attempt of the inner
insert resets the statement scope and pins again). Before this PR a branch
overwrite was exposed only when a concurrent commit moved the branch head onto
a snapshot with another schema; binding branch writes to the table's current
schema widened that to the same table-level changes as main.
The overwrite now records on the inner insert's
`PluginDrivenInsertCommandContext` the writer columns and write metadata
identity it normalized the plan against
(`InsertUtils.recordNormalizedWriteGeneration`). After the inner insert pins
again, `InsertUtils.checkNormalizedWriteGeneration` compares them (columns by
position: name, field ID, type, nullability and default) and fails the
statement with "retry the statement" when either differs, instead of binding
the expanded values by position to the new schema. This covers main and branch
overwrites alike, and an insert that normalizes its own plan is unaffected.
branch-4.2 avoids the hole differently: it carries the pinned
`IcebergWriteSchemaContext` on the unbound sink into the inner insert, so both
passes share one generation, and its commit fence rejects a later change with
"retry the statement". `InsertUtilsTest` replays the two pins with a reordered
schema and with a changed identity (both fail without the check) and with an u
nchanged generation.
--
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]
