joechenrh commented on code in PR #485:
URL: https://github.com/apache/arrow-go/pull/485#discussion_r2309552972


##########
parquet/file/column_reader.go:
##########
@@ -437,16 +446,27 @@ func (c *columnChunkReader) initDataDecoder(page Page, 
lvlByteLen int64) error {
                        format.Encoding_DELTA_LENGTH_BYTE_ARRAY,
                        format.Encoding_DELTA_BINARY_PACKED,
                        format.Encoding_BYTE_STREAM_SPLIT:
-                       c.curDecoder = 
c.decoderTraits.Decoder(parquet.Encoding(encoding), c.descr, false, c.mem)
-                       c.decoders[encoding] = c.curDecoder
+                       c.curDecoder = 
c.decoderTraits.Decoder(parquet.Encoding(enc), c.descr, false, c.mem)
+                       c.decoders[enc] = c.curDecoder
                case format.Encoding_RLE_DICTIONARY:
                        return errors.New("parquet: dictionary page must be 
before data page")
                default:
-                       return fmt.Errorf("parquet: unknown encoding type %s", 
encoding)
+                       return fmt.Errorf("parquet: unknown encoding type %s", 
enc)
+               }
+       }
+
+       switch c.descr.PhysicalType() {
+       case parquet.Types.FixedLenByteArray:
+               c.curDecoder = &encoding.FixedLenByteArrayDecoderWrapper{
+                       FixedLenByteArrayDecoder: 
c.curDecoder.(encoding.FixedLenByteArrayDecoder),
+               }
+       case parquet.Types.ByteArray:
+               c.curDecoder = &encoding.ByteArrayDecoderWrapper{
+                       ByteArrayDecoder: 
c.curDecoder.(encoding.ByteArrayDecoder),

Review Comment:
   I found another problem with the current `serializedPageReader`, either 
using memory allocator or buffer pool.
   
   For example, suppose each data page contains 4 `ByteArray`, and we want to 
read 12 data. We first read the data in 1st and 2nd page, store them in 
`values`. And these values may point to the internal buffer in the reader 
(depends on the decoder we use). If the pool/allocator give us a previously 
used buffer when reading the 3rd page, old values might be overwritten.
   
   So I add the wrapper here to make a copy of the output. Do you think it's 
acceptable? @zeroshade



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to