nssalian commented on code in PR #2002:
URL: https://github.com/apache/iceberg-go/pull/2002#discussion_r4040127971
##########
table/variant_residual.go:
##########
@@ -91,18 +196,138 @@ func buildExtractColumn(col iceberg.VariantExtractColumn,
rec arrow.RecordBatch,
}
if aerr := appendExtractLiteral(bldr, lit); aerr != nil {
- return nil, arrow.Field{}, aerr
+ return nil, aerr
}
}
- field := arrow.Field{
- Name: col.Name,
- Type: dt,
- Nullable: true,
- Metadata: arrow.NewMetadata([]string{ArrowParquetFieldIDKey},
[]string{strconv.Itoa(col.FieldID)}),
+ return bldr.NewArray(), nil
+}
+
+// tryShreddedTypedColumn returns the field's typed leaf column when it is
shredded to exactly dt, else nil.
+func tryShreddedTypedColumn(varr *extensions.VariantArray, path
variant.VariantPath, dt arrow.DataType, mem memory.Allocator) arrow.Array {
+ if path.Len() == 0 || varr.Data().Offset() != 0 {
+ return nil
+ }
+ tv := varr.Shredded()
+ if tv == nil || rootResidualHidesRows(varr, tv) {
+ return nil
+ }
+ n := varr.Len()
+
+ var mask *memory.Buffer
+ badOffset := false
+ mergeValidity := func(arr arrow.Array) {
+ if arr.Data().Offset() != 0 {
+ badOffset = true // child at a non-zero offset: our
offset-0 bit indexing would be wrong
+
+ return
+ }
+ if arr.NullN() == 0 {
+ return
+ }
+ vb := arr.Data().Buffers()[0]
+ if vb == nil {
+ return
+ }
+ if mask == nil {
+ mask = memory.NewResizableBuffer(mem)
+ mask.Resize(int(bitutil.BytesForBits(int64(n))))
+ copy(mask.Bytes(), vb.Bytes())
+
+ return
+ }
+ merged := bitutil.BitmapAndAlloc(mem, mask.Bytes(), vb.Bytes(),
0, 0, int64(n), 0)
+ mask.Release()
+ mask = merged
+ }
+ bail := func() arrow.Array {
+ if mask != nil {
+ mask.Release()
+ }
+
+ return nil
+ }
+
+ mergeValidity(varr.Storage())
+
+ cur := tv
+ for i := range path.Len() {
+ name, _, isField := path.StepAt(i)
+ if !isField {
+ return bail()
+ }
+ st, ok := cur.(*array.Struct)
+ if !ok {
+ return bail()
+ }
+ mergeValidity(st)
+ idx, ok := st.DataType().(*arrow.StructType).FieldIdx(name)
+ if !ok {
+ return bail()
+ }
+ field, ok := st.Field(idx).(*array.Struct)
+ if !ok {
+ return bail()
+ }
+ fty := field.DataType().(*arrow.StructType)
+ if vIdx, ok := fty.FieldIdx("value"); ok {
+ if v := field.Field(vIdx); v.NullN() != v.Len() {
+ return bail()
+ }
+ }
+ tvIdx, ok := fty.FieldIdx("typed_value")
+ if !ok {
+ return bail()
+ }
+ cur = field.Field(tvIdx)
Review Comment:
I had to re-read the spec again. You're right on the behavior. The spec
makes a null wrapper an absent key (the per-field group is required i.e. Java's
`ShreddedObjectReader` does `object.remove(name)` for a both-null field). So, I
folded `mergeValidity(field)` so the fast path returns null - no-op on
conformant data (`field.NullN()==0`), null on the non-conformant shape. Renamed
the test to `TestFastPathWrapperFieldNullIsAbsentKey` and made it assert null
(mutation-checked). arrow-go's per-row `VariantArray.Value` stays permissive
there
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
