zeroshade commented on code in PR #1937:
URL: https://github.com/apache/iceberg-go/pull/1937#discussion_r3883544240
##########
table/arrow_scanner.go:
##########
@@ -517,29 +636,87 @@ func readDeletes(ctx context.Context, fs iceio.IO,
dataFile iceberg.DataFile) (_
}
defer iceinternal.CheckedClose(rdr, &err)
- tbl, err := rdr.ReadTable(ctx)
+ schema, err := rdr.Schema()
+ if err != nil {
+ return nil, err
+ }
+
+ filePathIndex, posIndex, err := positionDeleteColumnIndices(schema)
if err != nil {
return nil, err
}
- defer tbl.Release()
- tbl, err = array.UnifyTableDicts(compute.GetAllocator(ctx), tbl)
+ tester, err := newPositionDeleteRowGroupTester(targets)
Review Comment:
**[P1] Validate the physical field-ID mapping before enabling this tester.**
Column projection intentionally resolves `file_path` and `pos` by name because
external files may omit Iceberg IDs, but the tester binds predicates to the
canonical reserved IDs and the stats/Bloom readers index physical columns by
their embedded IDs. With valid names/types but swapped reserved IDs, I
reproduced an unfiltered read returning position `0` while this target-filtered
read returned an empty map: `pos` statistics were interpreted as `file_path`
bounds and the applicable row group was pruned. That can expose a deleted row.
When IDs are present, require the canonical unique mapping (or return
`ErrInvalidSchema`); when IDs are absent, conservatively disable stats/Bloom
pruning. Please add swapped and duplicate-ID regressions.
--
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]
