minyoung commented on code in PR #33965:
URL: https://github.com/apache/arrow/pull/33965#discussion_r1093873932
##########
go/parquet/file/record_reader.go:
##########
@@ -751,14 +751,15 @@ type byteArrayRecordReader struct {
valueBuf []parquet.ByteArray
}
-func newByteArrayRecordReader(descr *schema.Column, info LevelInfo, mem
memory.Allocator, bufferPool *sync.Pool) RecordReader {
+func newByteArrayRecordReader(descr *schema.Column, info LevelInfo, dtype
arrow.DataType, mem memory.Allocator, bufferPool *sync.Pool) RecordReader {
if mem == nil {
mem = memory.DefaultAllocator
}
- dt := arrow.BinaryTypes.Binary
- if descr.LogicalType().Equals(schema.StringLogicalType{}) {
- dt = arrow.BinaryTypes.String
+ dt, ok := dtype.(arrow.BinaryDataType)
+ // arrow.DecimalType will also come through here, which we want to
treat as binary
+ if !ok {
+ dt = arrow.BinaryTypes.Binary
Review Comment:
> Could we instead use the metadata to determine whether or not we need to
use a LargeString/LargeBinary builder by checking how large the total data
column is?
This is a little tricky to do. We could potentially do
https://github.com/apache/arrow/commit/8b526e5afe5dc1b9f2a8b264e2693f406161cbf2,
but that's only summing the column chunk sizes and does not take into account
the column chunk dictionary. Imagine 2M rows each referencing the same 1k
string. The column chunk would have 1 string value in the dictionary, and all
the rows would use that dictionary value. At [decoding
time](https://github.com/apache/arrow/blob/go/v10.0.1/go/parquet/internal/utils/typed_rle_dict.gen.go#L1137),
the values would get filled out to >2G.
In the context of `pqarrow.WithStoreSchema()`, it might also be a little
surprising if the arrow schema says that a column is `arrow.LargeString`, but
you get an `arrow.String` back (since total data column size is low). Since
this specific change is only for `pqarrow.WithStoreSchema()`, so we could
revert this change and say that reads will always return `arrow.String`
--
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]
