tanmayrauth commented on code in PR #1337:
URL: https://github.com/apache/iceberg-go/pull/1337#discussion_r3486685796
##########
table/arrow_scanner.go:
##########
@@ -391,6 +391,32 @@ func groupPosDeletesByFilePath(ctx context.Context,
filePathCol, posCol *arrow.C
return results, nil
}
+func collectPosDeletePositions(positionalDeletes positionDeletes) (set[int64],
error) {
+ deletes := set[int64]{}
+ for _, chunk := range positionalDeletes {
+ if chunk == nil {
Review Comment:
Is the nil-chunk guard meant purely as defensiveness?
`groupPosDeletesByFilePath` only ever stores retained, non-nil chunks into the
map, so this branch shouldn't be reachable from the scan path — just want to
make sure I'm not missing a caller that can pass a nil chunk in.
##########
table/arrow_scanner.go:
##########
@@ -391,6 +391,32 @@ func groupPosDeletesByFilePath(ctx context.Context,
filePathCol, posCol *arrow.C
return results, nil
}
+func collectPosDeletePositions(positionalDeletes positionDeletes) (set[int64],
error) {
+ deletes := set[int64]{}
+ for _, chunk := range positionalDeletes {
+ if chunk == nil {
+ return nil, fmt.Errorf("%w: nil pos column chunk in
position delete file",
+ iceberg.ErrInvalidSchema)
+ }
+ if chunk.DataType().ID() != arrow.INT64 {
+ return nil, fmt.Errorf("%w: unsupported pos column type
%s in position delete file",
+ iceberg.ErrInvalidSchema, chunk.DataType())
+ }
+ for _, arr := range chunk.Chunks() {
Review Comment:
The inner `arr.(*array.Int64)` assertion is unreachable here — once the
outer `chunk.DataType().ID() == arrow.INT64` check at line 401 passes, every
chunk of that column is already `*array.Int64`, so the
`!ok` branch and its separate "unsupported pos chunk array type" message
can't fire. Not a problem to leave as defensive code, but you could drop the `,
ok` check (or fold the two error messages into one) to
keep it tight.
--
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]