github-actions[bot] commented on code in PR #65851:
URL: https://github.com/apache/doris/pull/65851#discussion_r3668677560
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergTransaction.java:
##########
@@ -145,6 +148,10 @@ public void beginInsert(ExternalTable dorisTable,
Optional<InsertCommandContext>
+ " is a tag, not a branch. Tags
cannot be targets for producing snapshots");
}
}
+ if (writeSchemaContext.isPresent()) {
+ writeSchemaContext.get().validateCurrentSchema(
+ table, insertCtx.isStaticPartitionOverwrite());
Review Comment:
[P0] Reject active-spec drift for dynamic INSERT OVERWRITE
This requires the pinned spec to remain current only for static overwrite.
For dynamic `INSERT OVERWRITE`, a retained partitioned P1 still passes after a
concurrent update makes unpartitioned P2 current. If the query returns no
files, `commitReplaceTxn()` then reads P2, takes its unpartitioned branch, and
deletes every existing file; under the P1 semantics used to plan the statement,
the same empty overwrite is a no-op. Require the fresh active spec to equal the
pinned spec for every overwrite (while allowing retained specs for append), and
test partitioned-P1 to unpartitioned-P2 drift with empty and non-empty results.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergTransaction.java:
##########
@@ -145,6 +148,10 @@ public void beginInsert(ExternalTable dorisTable,
Optional<InsertCommandContext>
+ " is a tag, not a branch. Tags
cannot be targets for producing snapshots");
}
}
+ if (writeSchemaContext.isPresent()) {
+ writeSchemaContext.get().validateCurrentSchema(
Review Comment:
[P1] Keep pinned-schema validation effective through commit replay
This check runs only before `newTransaction()`, but Iceberg 1.10.1 simple
transactions refresh and replay pending updates when the underlying metadata
changes, and the replayed snapshot is stamped with the refreshed current schema
ID. If another writer adds required field `r` with no initial default after
this check, Doris can stage an A-shaped file and commit it in a B-labeled
snapshot; the next current scan then rejects that file as missing `r`. The same
window affects INSERT, branch INSERT, UPDATE, and MERGE. Enforce the
pinned-schema/branch requirement atomically at commit or replay time, and test
a schema change after begin but before `commitTransaction()`.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -505,22 +524,63 @@ private List<String> getOrderedPathPartitionKeys() {
}
public void createScanRangeLocations() throws UserException {
- super.createScanRangeLocations();
+ Schema scanSchema = getQuerySchema();
+ Set<Integer> equalityDeleteFieldIds = Collections.emptySet();
+ if (!isSystemTable) {
+ equalityDeleteFieldIds = getEqualityDeleteFieldIdsForScan();
Review Comment:
[P1] Keep metadata-only COUNT(*) from planning every file
This preflight runs before `super.createScanRangeLocations()`, where
`isBatchMode()` can accept the snapshot-summary count when
`total-equality-deletes=0` and continue with only representative-task planning.
`loadEqualityDeleteFieldIds()` exhausts `scan.planFiles()` first, so `COUNT(*)`
on a large delete-free table now enumerates every applicable task, and this
work is outside the existing split-planning timers. Check the exact
summary-count eligibility before enumerating equality keys, and test that this
path skips the exhaustive equality preflight.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -505,22 +524,63 @@ private List<String> getOrderedPathPartitionKeys() {
}
public void createScanRangeLocations() throws UserException {
- super.createScanRangeLocations();
+ Schema scanSchema = getQuerySchema();
+ Set<Integer> equalityDeleteFieldIds = Collections.emptySet();
+ if (!isSystemTable) {
+ equalityDeleteFieldIds = getEqualityDeleteFieldIdsForScan();
+ boolean requiresCurrentSemantics =
requiresRecursiveInitialDefaultMaterialization(
+ scanSchema, desc.getSlots()) ||
!equalityDeleteFieldIds.isEmpty();
Review Comment:
[P1] Fence nested exact-name precedence during rolling upgrades
The new exact-current-name-before-alias lookup is enabled by the
pre-existing v1 semantics marker, so a source BE also accepts the FE's value 2
but still runs the old ordered lookup. With `renamed_payload` (ARRAY,
historical alias `payload`) plus replacement `payload` (MAP), current and
source BEs can assign different field IDs/types for the same projected nested
path. A scan with no recursive defaults, equality deletes, or missing required
fields passes all three predicates here and remains eligible for the source BE.
Detect this collision and require current BEs, or use a capability/version that
the source BE cannot mistake for v1, with a mixed-version planner test.
--
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]
