github-actions[bot] commented on code in PR #65851:
URL: https://github.com/apache/doris/pull/65851#discussion_r3633989268
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -504,17 +510,48 @@ private List<String> getOrderedPathPartitionKeys() {
}
public void createScanRangeLocations() throws UserException {
- super.createScanRangeLocations();
+ Schema scanSchema = getQuerySchema();
+ if (!isSystemTable) {
+ List<Column> projectedColumns = new ArrayList<>();
+ for (SlotDescriptor slot : desc.getSlots()) {
+ projectedColumns.add(slot.getColumn());
Review Comment:
[P1] Base the rolling-upgrade gate on the pruned projection
`slot.getColumn()` is the original full catalog column; Nereids stores the
pruned slot type and nested access paths separately. The helper therefore walks
unselected children too. For `s<a,b>` where only later-added `b` has an initial
default, `SELECT s.a` is rejected whenever a smooth-upgrade source exists even
though the BE projection never reads or materializes `b`. Please derive this
requirement from the slot's pruned type/access paths and add a planner-level
test for an unselected defaulted sibling.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -524,30 +561,178 @@ void enableCurrentIcebergScanSemantics() {
params.setIcebergScanSemanticsVersion(ICEBERG_SCAN_SEMANTICS_VERSION);
}
+ /**
+ * Build the schema metadata carrier used by both scanners and
equality-delete readers.
+ *
+ * <p>Batch-mode delete files are planned asynchronously after scan
parameters are sent to BE,
+ * so FE cannot first wait for their equality field IDs. Carry the latest
historical definition
+ * of every dropped primitive field; the field IDs referenced by any
planned equality delete are
+ * a subset of this bounded metadata. Iceberg field IDs are never reused.
+ */
+ @VisibleForTesting
+ List<NestedField> getSchemaFieldsForScan(Schema scanSchema) throws
UserException {
+ List<NestedField> fields = new ArrayList<>(scanSchema.columns());
+ if (isSystemTable) {
+ return fields;
+ }
+
+ Set<Integer> carriedFieldIds = new HashSet<>(
+ TypeUtil.indexById(scanSchema.asStruct()).keySet());
+ Preconditions.checkState(icebergTable instanceof HasTableOperations,
+ "Iceberg table does not expose metadata schema history: %s",
icebergTable.name());
+ List<Schema> schemaHistory = ((HasTableOperations) icebergTable)
+ .operations().current().schemas();
+ // Schema IDs may be reused when evolution returns to an earlier
schema, while the metadata
+ // list may also contain schemas committed after a time-travel or
branch target. Follow the
+ // actual scan snapshot's parent chain first so the field definition
active on that lineage
+ // wins. Then use the complete metadata list as a fallback for
schema-only changes and
+ // expired ancestors. A fallback definition may come from a later
rename, so BE resolves an
+ // ID-less equality key through the target mapping first and the
delete file's original key
+ // name second. Initial-default and field identity remain bound to the
stable field ID.
+ Snapshot snapshot = createTableScan().snapshot();
+ while (snapshot != null) {
+ Integer schemaId = snapshot.schemaId();
+ if (schemaId != null) {
+ Schema historicalSchema = icebergTable.schemas().get(schemaId);
+ Preconditions.checkState(historicalSchema != null,
+ "Iceberg snapshot schema %s is absent from table
metadata", schemaId);
+ addDroppedPrimitiveFields(fields, carriedFieldIds,
historicalSchema);
+ }
+ Long parentId = snapshot.parentId();
+ snapshot = parentId == null ? null :
icebergTable.snapshot(parentId);
+ }
+ for (int index = schemaHistory.size() - 1; index >= 0; index--) {
Review Comment:
[P1] Preserve the original equality-key name in the V2 fallback
When the selected snapshot's parent has expired, this reverse fallback can
choose a later rename (`k2`) for a dropped field before its target-relative
name (`k`); the new expired-parent test even constructs that state but checks
only the default. V1 handles it by retrying the delete file's `old_name`, yet
V2 BY_NAME only calls `find_column_by_name` with the carried `k2` metadata. For
an ID-less old file whose physical column is still `k`, V2 treats the key as
missing and synthesizes its initial default, so the equality delete can retain
or remove the wrong row. Please either exclude post-target schemas here or add
V2's non-authoritative retry through `filter.field_names[key_idx]`, with an
end-to-end V2 test that distinguishes a physical value from the default.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -504,17 +510,48 @@ private List<String> getOrderedPathPartitionKeys() {
}
public void createScanRangeLocations() throws UserException {
- super.createScanRangeLocations();
+ Schema scanSchema = getQuerySchema();
+ if (!isSystemTable) {
+ List<Column> projectedColumns = new ArrayList<>();
+ for (SlotDescriptor slot : desc.getSlots()) {
+ projectedColumns.add(slot.getColumn());
+ }
+ if (requiresRecursiveInitialDefaultMaterialization(scanSchema,
projectedColumns)) {
Review Comment:
[P1] Gate equality-key identity semantics on old source BEs
This compatibility check covers only projected recursive defaults, but the
patch also changes primitive equality-delete semantics in V1 by carrying
`_expand_col_field_ids`. During a rolling upgrade, current FE can still send a
drop/re-add-same-name key to a source BE at the PR base; that reader
reverse-scans `_id_to_block_column_name` by name, may select the replacement
field ID/default, and returns different rows from an upgraded BE. The existing
semantics version is unchanged, so it does not form a capability boundary for
this fix. Please also reject/exclude source BEs whenever the new historical
equality-key identity behavior may be needed, or introduce a distinct
advertised capability/version and test the mixed-version case.
--
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]
