You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/07/04 19:51:48 UTC
[PR] fix(arreflect): validate primitive conversion assignment overflow checks [arrow-go]
fallintoplace opened a new pull request, #903:
URL: https://github.com/apache/arrow-go/pull/903
Fixes arreflect primitive numeric conversions to reject out-of-range values before assignment via reflect (instead of truncating/wrapping).
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arreflect): validate primitive conversion assignment overflow checks [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #903:
URL: https://github.com/apache/arrow-go/pull/903#discussion_r3554123487
##########
arrow/array/arreflect/reflect_arrow_to_go.go:
##########
@@ -161,52 +161,92 @@ func setPrimitiveValue(v reflect.Value, arr arrow.Array, i int) error {
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int8 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(int64(arr.(*array.Int8).Value(i)))
+ val := int64(arr.(*array.Int8).Value(i))
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int8 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.INT16:
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int16 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(int64(arr.(*array.Int16).Value(i)))
+ val := int64(arr.(*array.Int16).Value(i))
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int16 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.INT32:
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int32 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(int64(arr.(*array.Int32).Value(i)))
+ val := int64(arr.(*array.Int32).Value(i))
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int32 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.INT64:
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int64 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(arr.(*array.Int64).Value(i))
+ val := arr.(*array.Int64).Value(i)
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int64 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.UINT8:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint8 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(uint64(arr.(*array.Uint8).Value(i)))
+ val := uint64(arr.(*array.Uint8).Value(i))
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint8 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.UINT16:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint16 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(uint64(arr.(*array.Uint16).Value(i)))
+ val := uint64(arr.(*array.Uint16).Value(i))
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint16 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.UINT32:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint32 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(uint64(arr.(*array.Uint32).Value(i)))
+ val := uint64(arr.(*array.Uint32).Value(i))
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint32 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.UINT64:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint64 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(arr.(*array.Uint64).Value(i))
+ val := arr.(*array.Uint64).Value(i)
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint64 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.FLOAT32:
if !isFloatKind(v.Kind()) {
return fmt.Errorf("cannot set float32 into %s: %w", v.Type(), ErrTypeMismatch)
}
+ val := float64(arr.(*array.Float32).Value(i))
+ if v.OverflowFloat(val) {
Review Comment:
Minor: for a FLOAT32 source this `OverflowFloat` check can never trigger — a float32 value widened to float64 always fits a float32 (or float64) destination, so it's effectively dead here, unlike the FLOAT64→float32 case below where it does real work. Harmless; feel free to drop it or leave a one-line note that it's kept for symmetry.
##########
arrow/array/arreflect/reflect_arrow_to_go.go:
##########
@@ -161,52 +161,92 @@ func setPrimitiveValue(v reflect.Value, arr arrow.Array, i int) error {
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int8 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(int64(arr.(*array.Int8).Value(i)))
+ val := int64(arr.(*array.Int8).Value(i))
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int8 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.INT16:
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int16 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(int64(arr.(*array.Int16).Value(i)))
+ val := int64(arr.(*array.Int16).Value(i))
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int16 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.INT32:
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int32 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(int64(arr.(*array.Int32).Value(i)))
+ val := int64(arr.(*array.Int32).Value(i))
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int32 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.INT64:
if !isIntKind(v.Kind()) {
return fmt.Errorf("cannot set int64 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetInt(arr.(*array.Int64).Value(i))
+ val := arr.(*array.Int64).Value(i)
+ if v.OverflowInt(val) {
+ return fmt.Errorf("cannot set int64 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetInt(val)
case arrow.UINT8:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint8 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(uint64(arr.(*array.Uint8).Value(i)))
+ val := uint64(arr.(*array.Uint8).Value(i))
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint8 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.UINT16:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint16 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(uint64(arr.(*array.Uint16).Value(i)))
+ val := uint64(arr.(*array.Uint16).Value(i))
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint16 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.UINT32:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint32 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(uint64(arr.(*array.Uint32).Value(i)))
+ val := uint64(arr.(*array.Uint32).Value(i))
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint32 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.UINT64:
if !isUintKind(v.Kind()) {
return fmt.Errorf("cannot set uint64 into %s: %w", v.Type(), ErrTypeMismatch)
}
- v.SetUint(arr.(*array.Uint64).Value(i))
+ val := arr.(*array.Uint64).Value(i)
+ if v.OverflowUint(val) {
+ return fmt.Errorf("cannot set uint64 value %d into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
+ v.SetUint(val)
case arrow.FLOAT32:
if !isFloatKind(v.Kind()) {
return fmt.Errorf("cannot set float32 into %s: %w", v.Type(), ErrTypeMismatch)
}
+ val := float64(arr.(*array.Float32).Value(i))
+ if v.OverflowFloat(val) {
+ return fmt.Errorf("cannot set float32 value %f into %s: %w", val, v.Type(), ErrTypeMismatch)
+ }
v.SetFloat(float64(arr.(*array.Float32).Value(i)))
Review Comment:
Minor/consistency: this re-reads `arr.(*array.Float32).Value(i)` instead of reusing the `val` computed just above (every other branch uses `v.SetInt(val)`/`v.SetUint(val)`/`v.SetFloat(val)`). Same result, just tidier — `v.SetFloat(val)`.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arreflect): validate primitive conversion assignment overflow checks [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #903:
URL: https://github.com/apache/arrow-go/pull/903
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org