fallintoplace commented on code in PR #1063:
URL: https://github.com/apache/arrow-go/pull/1063#discussion_r3685084349


##########
parquet/variant/variant.go:
##########
@@ -183,32 +183,38 @@ func (m *Metadata) loadDictionary(offsetSz uint8) error {
        }
 
        dictSize := readLEU32(m.data[hdrSizeBytes : hdrSizeBytes+offsetSz])
-       m.keys = make([][]byte, dictSize)
-
        if dictSize == 0 {
+               m.keys = nil
                return nil
        }
 
-       // first offset is always 0
-       offsetStart, offsetPos := uint32(0), hdrSizeBytes+offsetSz
-       valuesStart := hdrSizeBytes + (dictSize+2)*uint32(offsetSz)
-       if hdrSizeBytes+int(dictSize+1)*int(offsetSz) > len(m.data) {
-               return fmt.Errorf("%w: offset out of range: %d > %d",
-                       ErrInvalidMetadata, 
(dictSize+hdrSizeBytes)*uint32(offsetSz), len(m.data))
+       valuesStart := uint64(hdrSizeBytes) + 
(uint64(dictSize)+2)*uint64(offsetSz)
+       if valuesStart > uint64(len(m.data)) {

Review Comment:
   valuesStart == len(m.data) is valid when the offset table ends exactly at 
the input boundary and the dictionary’s strings contain zero total bytes.
   
   Changing > to >= would incorrectly reject that valid encoding. The 
subsequent check already rejects any nonzero final offset because valuesStart + 
end would exceed the input length.



-- 
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]

Reply via email to