zeroshade commented on code in PR #927:
URL: https://github.com/apache/arrow-go/pull/927#discussion_r3566723383
##########
arrow/scalar/nested.go:
##########
@@ -435,43 +435,60 @@ func (s *Dictionary) ValidateFull() (err error) {
return nil
}
- max := s.Value.Dict.Len() - 1
+ _, err = s.physicalIndex()
+ return
+}
+
+func (s *Dictionary) physicalIndex() (int, error) {
+ length := s.Value.Dict.Len()
+ outOfBounds := func(idx interface{}) error {
+ return fmt.Errorf("%s scalar index value out of bounds: %d",
s.DataType(), idx)
+ }
+
switch idx := s.Value.Index.value().(type) {
case int8:
- if idx < 0 || int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if idx < 0 || int64(idx) >= int64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
case uint8:
- if int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if uint64(idx) >= uint64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
case int16:
- if idx < 0 || int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if idx < 0 || int64(idx) >= int64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
case uint16:
- if int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if uint64(idx) >= uint64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
case int32:
- if idx < 0 || int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if idx < 0 || int64(idx) >= int64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
case uint32:
- if int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if uint64(idx) >= uint64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
case int64:
- if idx < 0 || int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if idx < 0 || idx >= int64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
case uint64:
- if int(idx) > max {
- err = fmt.Errorf("%s scalar index value out of bounds:
%d", s.DataType(), idx)
+ if idx >= uint64(length) {
+ return 0, outOfBounds(idx)
}
+ return int(idx), nil
+ default:
+ return 0, fmt.Errorf("unimplemented dictionary type %s",
s.DataType())
Review Comment:
Nit: report the index type here (`dt.IndexType`) rather than the whole
dictionary type, matching the old `GetEncodedValue` default. Also consider
adding an empty-dictionary (len == 0) out-of-range case to
`TestDictionaryScalarIndexBounds`.
--
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]
