laskoviymishka commented on code in PR #1151:
URL: https://github.com/apache/iceberg-go/pull/1151#discussion_r3395427657
##########
table/arrow_utils.go:
##########
@@ -1753,13 +1750,14 @@ func positionDeleteRecordsToDataFilesDV(ctx
context.Context, rootLocation string
positions := batch.Column(1).(*array.Int64)
for i := range batch.NumRows() {
- // PR (1) of #1135 reshaped DVWriter.Add to
take (specID,
- // partitionData) so the partitioned path can
pass through
- // partitionContext directly. This site is still
- // unpartitioned-only (the gate above routes
partitioned
- // writes elsewhere), so specID=0 + nil
partition. PR (2)
- // drops the gate and threads partitionContext
through here.
- writer.Add(filePaths.Value(int(i)),
[]int64{positions.Value(int(i))}, 0, nil)
+ filePath := filePaths.Value(int(i))
+ pCtx, ok := partitionContextByFilePath[filePath]
+ if !ok {
+ yield(nil, fmt.Errorf("unexpected
missing partition context for path %s", filePath))
Review Comment:
this change makes a nil `partitionContextByFilePath` invalid for any v3 row,
but `TestDVWritePathCancelledContext` still passes `nil` (it's unchanged in
this diff, around line 262 of the test file).
It passes today only because the cancelled iterator yields `(nil,
ctx.Err())` before this loop runs — so the lookup never happens. The three
other unpartitioned v3 tests in this PR were converted to
`unpartitionedContexts(...)`; this one was missed. If anyone later adds a valid
row before the cancellation, the failure mode silently flips from "propagates
context.Canceled" to "missing partition context." I'd switch it to
`unpartitionedContexts()` (or `map[string]partitionContext{}`) to match. Fix
that and this lands.
--
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]
