zeroshade commented on code in PR #1617:
URL: https://github.com/apache/iceberg-go/pull/1617#discussion_r3732106573
##########
table/equality_delete_reader.go:
##########
@@ -551,6 +558,10 @@ func processEqualityDeletesColumnar(ctx context.Context,
eqDeleteSets []*equalit
if len(indices) == 0 {
return nil, fmt.Errorf("equality delete
column %q not found in data record", name)
}
+ if len(indices) > 1 {
Review Comment:
This still false-accepts the opposite case. If the requested equality field
was renamed or differs only by case while another physical field uniquely has
the current table name, this code encodes that other field's values as delete
keys without confirming its ID. That can silently delete the wrong rows.
Suggested fix: carry `eqDel.fieldIDs` into this path and resolve each
data-record column by physical field ID, requiring exactly one match; use
unique-name fallback only when ID metadata is absent. Include the equality
field ID and data-file path in the error.
##########
table/equality_delete_reader.go:
##########
@@ -217,11 +220,15 @@ func readEqualityDeleteFile(ctx context.Context, fs
iceio.IO, tableSchema *icebe
colNames[i] = name
indices := tbl.Schema().FieldIndices(name)
- if len(indices) == 0 {
+ switch len(indices) {
Review Comment:
This name-based ambiguity check false-rejects a valid, ID-unambiguous file.
Two physical fields can have the same name but distinct IDs while
`equality_ids` identifies exactly one; production Parquet-to-Arrow schemas
retain `PARQUET:field_id`, so the authoritative identity is available here.
`FieldIndices` is also exact, case-sensitive, and top-level-only, so it cannot
resolve nested equality IDs represented as dotted Iceberg paths.
Suggested fix: recursively resolve the physical field ID first and require
exactly one ID match, using the metadata visitor support in
`table/arrow_utils.go`; use a unique exact-name fallback only when ID metadata
is absent. Include the field ID in missing/ambiguous errors.
--
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]
