github-actions[bot] commented on code in PR #68267:
URL: https://github.com/apache/doris/pull/68267#discussion_r4068073005
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -1768,7 +1768,17 @@ Status
IcebergTableReader::_resolve_equality_delete_fields(
return Status::NotSupported(
"Iceberg equality delete does not support complex column
{}", field->name);
}
- const auto key_type = path.size() > 1 ? make_nullable(field->type) :
field->type;
+ // Equality comparison must run in the promoted (current snapshot
schema) type domain.
+ // Narrowing a wider data key into a historical delete-file type is
lossy: an INT overflow
+ // becomes NULL, and NULL-safe equality then matches a NULL delete
key. Delete values are
+ // promoted into this same domain when the delete file is read.
+ const DataTypePtr delete_file_type =
+ path.size() > 1 ? make_nullable(field->type) : field->type;
+ DataTypePtr key_type = delete_file_type;
+ if (auto table_field = _find_table_column_by_field_id(field_id,
delete_file_type, true);
Review Comment:
[P1] Gate promoted delete keys on the negotiated scan semantics
This promotion runs even when `iceberg_scan_semantics_version` is absent.
Old FEs already send `current_schema_id` plus typed `history_schema_info` (see
the legacy-plan test), but `iceberg_scan_semantics.h` explicitly says that only
the version marker may opt a new BE into result-changing semantics. In a
BE-first rolling upgrade, the PR's INT-delete/BIGINT-data case therefore
differs by backend: an old BE narrows `4294967296` into the INT domain and
deletes it (or errors in strict mode), while a new BE promotes the delete key
and retains it. A distributed scan can consequently return backend-dependent
rows. Please gate this branch on
`supports_iceberg_scan_semantics_v2(_scan_params)` and preserve the old
delete-file domain when the marker is absent; add a no-marker regression beside
the v2 test.
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -1768,7 +1768,17 @@ Status
IcebergTableReader::_resolve_equality_delete_fields(
return Status::NotSupported(
"Iceberg equality delete does not support complex column
{}", field->name);
}
- const auto key_type = path.size() > 1 ? make_nullable(field->type) :
field->type;
+ // Equality comparison must run in the promoted (current snapshot
schema) type domain.
Review Comment:
[P1] Keep the selectable legacy scanner in the promoted domain
This makes the comparison widening true only in FileScannerV2.
`enable_file_scanner_v2=false` is still a supported Iceberg path:
`should_use_file_scanner_v2` routes it to `IcebergReaderMixin`, whose Parquet
and ORC equality-delete builders derive the key type from the delete file and
reuse it for the data-file expand column. With this test's
INT-delete/BIGINT-data values, that path still narrows `4294967296` to nullable
INT and can delete it by matching the NULL key, so toggling the session option
changes query results. Please apply the semantics-gated promoted comparison
domain to the legacy mixin as well, or force affected equality-delete scans
onto v2, and add an `enable_file_scanner_v2=false` parity regression.
--
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]
