fallintoplace commented on code in PR #1229:
URL: https://github.com/apache/iceberg-go/pull/1229#discussion_r3461898206
##########
table/arrow_scanner.go:
##########
@@ -277,29 +430,8 @@ func readDeletes(ctx context.Context, fs iceio.IO,
dataFile iceberg.DataFile) (_
filePathCol :=
tbl.Column(tbl.Schema().FieldIndices("file_path")[0]).Data()
posCol := tbl.Column(tbl.Schema().FieldIndices("pos")[0]).Data()
- dict :=
filePathCol.Chunk(0).(*array.Dictionary).Dictionary().(*array.String)
Review Comment:
I don’t think we can assume the Unique result is always dictionary-encoded.
My understanding is that unique returns a single ArrayDatum because
OutputChunked=false, but the array’s concrete type follows the input layout:
plain string / large string inputs can produce a string-like array, while
dictionary input produces a dictionary array. Since the main bug here is
plain-string file_path layouts, the dict-only wrapper path would miss or panic
on the case this PR is fixing.
I’m happy to keep the simplification around assuming ArrayDatum, but I think
we still need the array-level type switch for array.StringLike vs
*array.Dictionary.
--
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]
