zeroshade commented on code in PR #1161:
URL: https://github.com/apache/arrow-go/pull/1161#discussion_r3832330661
##########
parquet/pqarrow/encode_arrow.go:
##########
@@ -121,11 +121,58 @@ func targetDecimalPrecision(cw file.ColumnChunkWriter)
(int32, bool) {
}
func decimal128FitsTargetPrecision(val decimal128.Num, precision int32) bool {
- return precision > 0 && precision <= decimal128.MaxPrecision &&
val.FitsInPrecision(precision)
+ if precision <= 0 || precision > decimal128.MaxPrecision {
+ return false
+ }
+ // Abs overflows for the minimum two's-complement value.
+ if val.Sign() < 0 && val.Negate() == val {
+ return false
+ }
+ return val.FitsInPrecision(precision)
}
func decimal256FitsTargetPrecision(val decimal256.Num, precision int32) bool {
- return precision > 0 && precision <= decimal256.MaxPrecision &&
val.FitsInPrecision(precision)
+ if precision <= 0 || precision > decimal256.MaxPrecision {
+ return false
+ }
+ // Abs overflows for the minimum two's-complement value.
+ if val.Sign() < 0 && val.Negate() == val {
+ return false
+ }
+ return val.FitsInPrecision(precision)
+}
+
+// validatePresentDecimalValues visits only values represented by a definition
+// level. Values below the repeated ancestor definition level are list
+// placeholders and do not have a corresponding entry in arr.
+func validatePresentDecimalValues(arr arrow.Array, defLevels []int16,
levelInfo file.LevelInfo, validate func(int) error) error {
+ if len(defLevels) == 0 {
+ for idx := 0; idx < arr.Len(); idx++ {
+ if arr.IsValid(idx) {
+ if err := validate(idx); err != nil {
+ return err
+ }
+ }
+ }
+ return nil
+ }
+
+ valueIdx := 0
+ for _, defLevel := range defLevels {
+ if defLevel < levelInfo.RepeatedAncestorDefLevel {
Review Comment:
Could we add a regression test exercising this repeated-ancestor branch? The
new tests use flat or struct-only schemas, so `RepeatedAncestorDefLevel`
remains zero. A case such as `list<decimal(4,0)>` with `[[], [99999], [1]]`
would verify that a leading empty list does not misalign `valueIdx` and allow
the overflowing value to escape validation. This is non-blocking.
--
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]
