github-actions[bot] commented on code in PR #65851:
URL: https://github.com/apache/doris/pull/65851#discussion_r3632994281
##########
be/src/format/table/iceberg_reader.cpp:
##########
@@ -301,7 +301,7 @@ Status
IcebergParquetReader::on_before_init_reader(ReaderInitContext* ctx) {
}
} else {
std::string table_column_name = old_name;
- if (const auto* table_field = _find_current_schema_field(field_id);
+ if (const auto* table_field = _find_schema_field(field_id);
Review Comment:
[P1] Keep equality keys keyed by field ID when names are reused
This new historical lookup still receives `field_id` from the reverse name
scan immediately above. If an equality key `k` (ID 7, initial default 7) is
dropped and a new projected `k` (ID 8, default 9) is added,
`_id_to_block_column_name` contains both `7 -> k` (from the delete file) and `8
-> k` (from the slot). The unordered scan at lines 285-293 can therefore choose
ID 8; for an old data file containing neither field, this call and
`_register_missing_equality_delete_column()` synthesize 9, so the ID-7 equality
delete no longer removes the row. ORC has the same reverse-by-name lookup.
Please carry the delete field ID alongside each expand column instead of
recovering it from a non-unique name (and add the drop/re-add-same-name case
for both V1 formats).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -524,30 +560,153 @@ 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) {
+ List<NestedField> fields = new ArrayList<>(scanSchema.columns());
+ if (isSystemTable) {
+ return fields;
+ }
+
+ Set<Integer> carriedFieldIds = new HashSet<>(
+ TypeUtil.indexById(scanSchema.asStruct()).keySet());
+ List<Integer> schemaIds = new
ArrayList<>(icebergTable.schemas().keySet());
+ schemaIds.sort(Collections.reverseOrder());
Review Comment:
[P1] Preserve schema-list chronology for dropped fields
Iceberg schema IDs are unique identifiers, not an evolution sequence, so
sorting them numerically can select an older definition for a dropped ID. For
example, with schema IDs `100: k` (field 7, initial default 7), then `1: rename
k2`, then `2: drop`, this carrier retains stale name `k`. In Doris's supported
BY_NAME fallback for an ID-less file without an explicit name mapping, a
physical `k2` written after the rename no longer binds; the reader materializes
7, so an equality delete for field 7/value 9 fails to remove a row with `k2 =
9`. Please recover the definition immediately before the drop from actual
schema-list chronology, and add this non-monotonic-ID rename/delete/drop 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]
