zeroshade commented on code in PR #930:
URL: https://github.com/apache/arrow-go/pull/930#discussion_r3566723253
##########
arrow/array/validate.go:
##########
@@ -33,32 +36,390 @@ type Validator interface {
}
// Validate performs a basic O(1) consistency check on arr, returning an error
-// if the array's internal buffers are inconsistent. For array types that do
not
-// implement Validator, nil is returned.
+// if the array's internal buffers or nested child data are inconsistent.
//
// Use this to detect corrupted data from untrusted sources such as Arrow
Flight
// or Flight SQL servers before accessing values, which may otherwise panic.
func Validate(arr arrow.Array) error {
- if v, ok := arr.(Validator); ok {
- return v.Validate()
- }
- return nil
+ return validateArray(arr, false, "")
}
// ValidateFull performs a thorough O(n) consistency check on arr, returning an
-// error if the array's internal buffers are inconsistent. For array types that
-// do not implement Validator, nil is returned.
+// error if the array's internal buffers or nested child data are inconsistent.
//
// Unlike Validate, this checks every element and is therefore O(n). Use this
// when receiving data from untrusted sources where subtle corruption (e.g.
// non-monotonic offsets) may not be detected by Validate alone.
func ValidateFull(arr arrow.Array) error {
+ return validateArray(arr, true, "")
+}
+
+func validateArray(arr arrow.Array, full bool, path string) error {
+ if arr == nil {
+ return nil
+ }
+
+ data, ok := arr.Data().(*Data)
+ if !ok || data == nil {
+ return validationError(path, fmt.Errorf("arrow/array: array
does not expose internal data"))
+ }
+
+ if err := validateArrayData(data); err != nil {
+ return validationError(path, err)
+ }
+ if err := validateArrayStructure(data); err != nil {
+ return validationError(path, err)
+ }
+
if v, ok := arr.(Validator); ok {
- return v.ValidateFull()
+ var err error
+ if full {
+ err = v.ValidateFull()
+ } else {
+ err = v.Validate()
+ }
+ if err != nil {
+ return validationError(path, err)
+ }
+ }
+
+ if data.DataType().Layout().HasDict {
+ dictData := data.dictionary
+ if dictData != nil {
+ dt := data.DataType().(*arrow.DictionaryType)
+ indexData := NewData(dt.IndexType, data.length,
data.buffers, nil, data.nulls, data.offset)
+ err := checkIndexBounds(indexData,
uint64(dictData.Len()))
+ indexData.Release()
+ if err != nil {
+ return validationError(joinValidationPath(path,
"dictionary indices"), err)
+ }
+ }
}
+
+ if ext, ok := arr.(ExtensionArray); ok {
+ return validateArray(ext.Storage(), full,
joinValidationPath(path, "storage"))
+ }
+
+ for i, childData := range data.Children() {
+ child, err := makeArrayFromData(childData)
+ if err != nil {
+ return validationError(joinValidationPath(path,
validationChildPath(data.DataType(), i)), err)
+ }
+
+ childPath := joinValidationPath(path,
validationChildPath(data.DataType(), i))
+ err = validateArray(child, full, childPath)
+ child.Release()
+ if err != nil {
+ return err
+ }
+ }
+
+ if dictData := data.dictionary; dictData != nil {
+ dict, err := makeArrayFromData(dictData)
+ if err != nil {
+ return validationError(joinValidationPath(path,
"dictionary"), err)
+ }
+
+ err = validateArray(dict, full, joinValidationPath(path,
"dictionary"))
+ dict.Release()
+ if err != nil {
+ return err
+ }
+ }
+
return nil
}
+func validateArrayData(data *Data) error {
+ if data == nil || data.dtype == nil {
+ return fmt.Errorf("arrow/array: array data has no data type")
+ }
+ if data.offset < 0 {
+ return fmt.Errorf("arrow/array: array offset is negative: %d",
data.offset)
+ }
+ if data.length < 0 {
+ return fmt.Errorf("arrow/array: array length is negative: %d",
data.length)
+ }
+ if data.nulls < UnknownNullCount || data.nulls > data.length {
+ return fmt.Errorf("arrow/array: invalid null count %d for
length %d", data.nulls, data.length)
+ }
+
+ end := int64(data.offset) + int64(data.length)
+ if end < int64(data.offset) {
+ return fmt.Errorf("arrow/array: array offset and length
overflow")
+ }
+
+ layout := data.dtype.Layout()
+ if len(data.buffers) < len(layout.Buffers) {
+ return fmt.Errorf("arrow/array: expected at least %d buffers
for %s, got %d",
+ len(layout.Buffers), data.dtype, len(data.buffers))
+ }
+ if layout.VariadicSpec == nil && len(data.buffers) >
len(layout.Buffers) {
Review Comment:
Blocking: this rejects valid sparse/dense unions. `validateArray` runs
`validateArrayData` for every array, and unions carry a leading `nil` validity
placeholder in `Data` (sparse `[nil, typeIDs]`, dense `[nil, typeIDs,
valueOffsets]`) that `UnionType.Layout()` omits (sparse = 1 buffer, dense = 2).
So a valid sparse union has 2 buffers vs 1 expected (dense 3 vs 2) and fails
here with the `expected at most N buffers` error — both at the top level and as
a nested child. Please account for the union validity-slot convention (e.g.
skip the leading nil for union layouts).
--
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]
