From c2026ab8001a0e2a634e6645eb061890a5d675ab Mon Sep 17 00:00:00 2001 From: Curt Hagenlocher Date: Sat, 26 Sep 2026 21:23:32 -0700 Subject: [PATCH 1/3] fix: Read large_binary and binary_view storage in shredded variant readers The shredded readers cast value columns to BinaryArray and string/binary typed_value columns to StringArray/BinaryArray, throwing for the other storage types VariantArray and ShredSchema accept. Read through helpers that handle every binary and string representation instead. Co-Authored-By: Claude Opus 5.5 --- .../Shredding/ShreddedArray.cs | 5 +- .../Shredding/ShreddedObject.cs | 8 +- .../Shredding/ShreddedVariant.cs | 10 +- .../Shredding/ShreddingHelpers.cs | 34 ++++ .../ShreddedVariantStorageTypeTests.cs | 192 ++++++++++++++++++ 5 files changed, 235 insertions(+), 14 deletions(-) create mode 100644 test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs diff --git a/src/Apache.Arrow.Operations/Shredding/ShreddedArray.cs b/src/Apache.Arrow.Operations/Shredding/ShreddedArray.cs index 2dcea000..e1d5956f 100644 --- a/src/Apache.Arrow.Operations/Shredding/ShreddedArray.cs +++ b/src/Apache.Arrow.Operations/Shredding/ShreddedArray.cs @@ -106,7 +106,7 @@ public bool TryGetResidualReader(out VariantReader reader) reader = default; return false; } - ReadOnlySpan bytes = ((BinaryArray)_residual).GetBytes(_row, out _); + ReadOnlySpan bytes = ShreddingHelpers.GetBytes(_residual, _row); reader = new VariantReader(_metadata, bytes); return true; } @@ -145,8 +145,7 @@ public VariantValue ToVariantValue() { return VariantValue.Null; } - BinaryArray residualBinary = (BinaryArray)_residual; - ReadOnlySpan bytes = residualBinary.GetBytes(_row, out _); + ReadOnlySpan bytes = ShreddingHelpers.GetBytes(_residual, _row); return new VariantReader(_metadata, bytes).ToVariantValue(); } } diff --git a/src/Apache.Arrow.Operations/Shredding/ShreddedObject.cs b/src/Apache.Arrow.Operations/Shredding/ShreddedObject.cs index 07e23811..98a06514 100644 --- a/src/Apache.Arrow.Operations/Shredding/ShreddedObject.cs +++ b/src/Apache.Arrow.Operations/Shredding/ShreddedObject.cs @@ -104,7 +104,7 @@ public bool TryGetResidualReader(out VariantReader reader) reader = default; return false; } - ReadOnlySpan bytes = ((BinaryArray)_residual).GetBytes(_index, out _); + ReadOnlySpan bytes = ShreddingHelpers.GetBytes(_residual, _index); reader = new VariantReader(_metadata, bytes); return true; } @@ -128,8 +128,7 @@ public VariantValue ToVariantValue() // No shredded fields at this row — whatever is in the residual IS the value. if (!typedPopulated) { - BinaryArray binary = (BinaryArray)_residual; - ReadOnlySpan bytes = binary.GetBytes(_index, out _); + ReadOnlySpan bytes = ShreddingHelpers.GetBytes(_residual, _index); return new VariantReader(_metadata, bytes).ToVariantValue(); } @@ -151,8 +150,7 @@ public VariantValue ToVariantValue() // Partially shredded object — merge residual unshredded fields. if (residualPopulated) { - BinaryArray residualBinary = (BinaryArray)_residual; - ReadOnlySpan residualBytes = residualBinary.GetBytes(_index, out _); + ReadOnlySpan residualBytes = ShreddingHelpers.GetBytes(_residual, _index); VariantReader residualReader = new VariantReader(_metadata, residualBytes); if (!residualReader.IsObject) { diff --git a/src/Apache.Arrow.Operations/Shredding/ShreddedVariant.cs b/src/Apache.Arrow.Operations/Shredding/ShreddedVariant.cs index 5ef35759..3b58a750 100644 --- a/src/Apache.Arrow.Operations/Shredding/ShreddedVariant.cs +++ b/src/Apache.Arrow.Operations/Shredding/ShreddedVariant.cs @@ -125,8 +125,7 @@ public bool TryGetResidualReader(out VariantReader reader) { if (HasResidual) { - BinaryArray binary = (BinaryArray)_valueArray; - ReadOnlySpan bytes = binary.GetBytes(_index, out _); + ReadOnlySpan bytes = ShreddingHelpers.GetBytes(_valueArray, _index); reader = new VariantReader(_metadata, bytes); return true; } @@ -168,8 +167,7 @@ private VariantValue ReadResidual() { throw new InvalidOperationException("No residual value to read."); } - BinaryArray binary = (BinaryArray)_valueArray; - ReadOnlySpan bytes = binary.GetBytes(_index, out _); + ReadOnlySpan bytes = ShreddingHelpers.GetBytes(_valueArray, _index); return new VariantReader(_metadata, bytes).ToVariantValue(); } @@ -266,10 +264,10 @@ public SqlDecimal GetSqlDecimal() public long GetTimestampNtzNanos() => ((TimestampArray)RequireTyped(ShredType.TimestampNtzNanos)).GetValue(_index).Value; /// Reads the shredded string value at this slot. - public string GetString() => ((StringArray)RequireTyped(ShredType.String)).GetString(_index); + public string GetString() => ShreddingHelpers.GetString(RequireTyped(ShredType.String), _index); /// Reads the shredded binary value at this slot as a byte span. - public ReadOnlySpan GetBinaryBytes() => ((BinaryArray)RequireTyped(ShredType.Binary)).GetBytes(_index); + public ReadOnlySpan GetBinaryBytes() => ShreddingHelpers.GetBytes(RequireTyped(ShredType.Binary), _index); /// Reads the shredded UUID at this slot. public Guid GetUuid() diff --git a/src/Apache.Arrow.Operations/Shredding/ShreddingHelpers.cs b/src/Apache.Arrow.Operations/Shredding/ShreddingHelpers.cs index f7be85e8..36b628e4 100644 --- a/src/Apache.Arrow.Operations/Shredding/ShreddingHelpers.cs +++ b/src/Apache.Arrow.Operations/Shredding/ShreddingHelpers.cs @@ -44,5 +44,39 @@ public static ShreddedVariant BuildSlot( return new ShreddedVariant(slotSchema, metadata, valueArr, typedArr, index); } + + /// + /// Reads the bytes at from any binary representation a + /// variant column may use (binary, large_binary or binary_view). + /// + public static ReadOnlySpan GetBytes(IArrowArray array, int index) + { + switch (array) + { + case BinaryArray binary: return binary.GetBytes(index); + case LargeBinaryArray largeBinary: return largeBinary.GetBytes(index); + case BinaryViewArray binaryView: return binaryView.GetBytes(index); + default: + throw new InvalidOperationException( + $"Cannot read variant bytes from an array of type {array.Data.DataType.TypeId}."); + } + } + + /// + /// Reads the string at from any string representation + /// (utf8, large_utf8 or utf8_view). + /// + public static string GetString(IArrowArray array, int index) + { + switch (array) + { + case StringArray str: return str.GetString(index); + case LargeStringArray largeStr: return largeStr.GetString(index); + case StringViewArray strView: return strView.GetString(index); + default: + throw new InvalidOperationException( + $"Cannot read a string from an array of type {array.Data.DataType.TypeId}."); + } + } } } diff --git a/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs b/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs new file mode 100644 index 00000000..e4269a89 --- /dev/null +++ b/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs @@ -0,0 +1,192 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System; +using System.Collections.Generic; +using Apache.Arrow; +using Apache.Arrow.Operations.Shredding; +using Apache.Arrow.Scalars.Variant; +using Apache.Arrow.Types; +using Xunit; + +namespace Apache.Arrow.Operations.Tests.Shredding +{ + /// + /// The shredded readers must accept every binary representation a variant column + /// may use for its metadata and value fields (binary, large_binary, + /// binary_view), at every nesting level, as well as large_utf8 / large_binary + /// typed_value columns. + /// + public class ShreddedVariantStorageTypeTests + { + public enum Storage { LargeBinary, BinaryView } + + private static readonly ShredSchema Schema = ShredSchema.ForObject(new Dictionary + { + ["a"] = ShredSchema.Primitive(ShredType.Int32), + ["s"] = ShredSchema.Primitive(ShredType.String), + ["bin"] = ShredSchema.Primitive(ShredType.Binary), + ["tags"] = ShredSchema.ForArray(ShredSchema.Primitive(ShredType.Int32)), + }); + + private static VariantValue Obj(params (string Name, VariantValue Value)[] fields) + { + var dict = new Dictionary(); + foreach (var (name, value) in fields) dict[name] = value; + return VariantValue.FromObject(dict); + } + + // Exercises residuals at the top level, inside a partially shredded object, + // inside an object field element group, and inside a list element group. + private static readonly List Rows = new List + { + Obj(("a", VariantValue.FromInt32(1)), + ("s", VariantValue.FromString("hello")), + ("bin", VariantValue.FromBinary(new byte[] { 1, 2, 3 })), + ("tags", VariantValue.FromArray(VariantValue.FromInt32(7), VariantValue.FromString("residual element"))), + ("extra", VariantValue.FromBoolean(true))), + null, + VariantValue.FromString("not an object"), + Obj(("a", VariantValue.FromString("residual field")), + ("tags", VariantValue.FromString("not an array"))), + }; + + private static VariantArray BuildShredded() + { + (byte[] metadata, IReadOnlyList rows) = VariantShredder.Shred(Rows, Schema); + return ShreddedVariantArrayBuilder.Build(Schema, metadata, rows); + } + + [Theory] + [InlineData(Storage.LargeBinary)] + [InlineData(Storage.BinaryView)] + public void GetLogicalVariantValue_ReadsAlternateStorage(Storage storage) + { + VariantArray array = Convert(BuildShredded(), storage); + + Assert.Equal(Rows.Count, array.Length); + for (int i = 0; i < Rows.Count; i++) + { + if (Rows[i].HasValue) + Assert.Equal(Rows[i].Value, array.GetLogicalVariantValue(i)); + else + Assert.True(array.IsNull(i)); + } + } + + [Theory] + [InlineData(Storage.LargeBinary)] + [InlineData(Storage.BinaryView)] + public void GetLogicalVariantValue_ReadsAlternateStorage_Unshredded(Storage storage) + { + var builder = new VariantArray.Builder(); + builder.AppendRange(Rows); + VariantArray array = Convert(builder.Build(), storage); + + Assert.Equal(Rows[0].Value, array.GetLogicalVariantValue(0)); + } + + [Fact] + public void TypedAccessors_ReadLargeTypedColumns() + { + VariantArray array = Convert(BuildShredded(), Storage.LargeBinary); + ShreddedObject obj = array.GetShreddedVariant(0).GetObject(); + + Assert.True(obj.TryGetField("s", out ShreddedVariant s)); + Assert.Equal("hello", s.GetString()); + Assert.True(obj.TryGetField("bin", out ShreddedVariant bin)); + Assert.Equal(new byte[] { 1, 2, 3 }, bin.GetBinaryBytes().ToArray()); + } + + // --------------------------------------------------------------- + // Storage conversion + // --------------------------------------------------------------- + + private static VariantArray Convert(VariantArray array, Storage storage) + { + return new VariantArray((StructArray)ConvertArray(array.StorageArray, null, storage)); + } + + /// + /// Recursively rewrites metadata / value binary fields to the target + /// storage. For it also rewrites string and binary + /// typed_value columns to large_utf8 / large_binary; there is no view + /// counterpart because the shredding schema doesn't map view types for typed_value. + /// + private static IArrowArray ConvertArray(IArrowArray array, string fieldName, Storage storage) + { + bool isVariantBinary = fieldName == "metadata" || fieldName == "value"; + switch (array) + { + case StringArray str when !isVariantBinary && storage == Storage.LargeBinary: + { + var b = new LargeStringArray.Builder(); + for (int i = 0; i < str.Length; i++) + { + if (str.IsNull(i)) b.AppendNull(); else b.Append(str.GetString(i)); + } + return b.Build(); + } + case StringArray str: + return str; + case BinaryArray bin when isVariantBinary || storage == Storage.LargeBinary: + return storage == Storage.LargeBinary ? ToLargeBinary(bin) : (IArrowArray)ToBinaryView(bin); + case StructArray st: + { + var type = (StructType)st.Data.DataType; + var fields = new List(); + var children = new List(); + for (int f = 0; f < type.Fields.Count; f++) + { + Field field = type.Fields[f]; + IArrowArray child = ConvertArray(st.Fields[f], field.Name, storage); + children.Add(child); + fields.Add(new Field(field.Name, child.Data.DataType, field.IsNullable)); + } + return new StructArray(new StructType(fields), st.Length, children, st.NullBitmapBuffer, st.NullCount); + } + case ListArray list: + { + Field element = ((ListType)list.Data.DataType).ValueField; + IArrowArray values = ConvertArray(list.Values, element.Name, storage); + var listType = new ListType(new Field(element.Name, values.Data.DataType, element.IsNullable)); + return new ListArray(listType, list.Length, list.ValueOffsetsBuffer, values, list.NullBitmapBuffer, list.NullCount); + } + default: + return array; + } + } + + private static LargeBinaryArray ToLargeBinary(BinaryArray bin) + { + var b = new LargeBinaryArray.Builder(); + for (int i = 0; i < bin.Length; i++) + { + if (bin.IsNull(i)) b.AppendNull(); else b.Append(bin.GetBytes(i)); + } + return b.Build(); + } + + private static BinaryViewArray ToBinaryView(BinaryArray bin) + { + var b = new BinaryViewArray.Builder(); + for (int i = 0; i < bin.Length; i++) + { + if (bin.IsNull(i)) b.AppendNull(); else b.Append(bin.GetBytes(i)); + } + return b.Build(); + } + } +} From 0d2dfe563f5b77cf1645f971b8d212c2dc43759f Mon Sep 17 00:00:00 2001 From: Curt Hagenlocher Date: Sun, 27 Sep 2026 09:14:53 -0700 Subject: [PATCH 2/3] style: Apply dotnet format to storage type tests Co-Authored-By: Claude Opus 5.5 --- .../ShreddedVariantStorageTypeTests.cs | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs b/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs index e4269a89..9f268858 100644 --- a/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs +++ b/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs @@ -131,39 +131,39 @@ private static IArrowArray ConvertArray(IArrowArray array, string fieldName, Sto switch (array) { case StringArray str when !isVariantBinary && storage == Storage.LargeBinary: - { - var b = new LargeStringArray.Builder(); - for (int i = 0; i < str.Length; i++) { - if (str.IsNull(i)) b.AppendNull(); else b.Append(str.GetString(i)); + var b = new LargeStringArray.Builder(); + for (int i = 0; i < str.Length; i++) + { + if (str.IsNull(i)) b.AppendNull(); else b.Append(str.GetString(i)); + } + return b.Build(); } - return b.Build(); - } case StringArray str: return str; case BinaryArray bin when isVariantBinary || storage == Storage.LargeBinary: return storage == Storage.LargeBinary ? ToLargeBinary(bin) : (IArrowArray)ToBinaryView(bin); case StructArray st: - { - var type = (StructType)st.Data.DataType; - var fields = new List(); - var children = new List(); - for (int f = 0; f < type.Fields.Count; f++) { - Field field = type.Fields[f]; - IArrowArray child = ConvertArray(st.Fields[f], field.Name, storage); - children.Add(child); - fields.Add(new Field(field.Name, child.Data.DataType, field.IsNullable)); + var type = (StructType)st.Data.DataType; + var fields = new List(); + var children = new List(); + for (int f = 0; f < type.Fields.Count; f++) + { + Field field = type.Fields[f]; + IArrowArray child = ConvertArray(st.Fields[f], field.Name, storage); + children.Add(child); + fields.Add(new Field(field.Name, child.Data.DataType, field.IsNullable)); + } + return new StructArray(new StructType(fields), st.Length, children, st.NullBitmapBuffer, st.NullCount); } - return new StructArray(new StructType(fields), st.Length, children, st.NullBitmapBuffer, st.NullCount); - } case ListArray list: - { - Field element = ((ListType)list.Data.DataType).ValueField; - IArrowArray values = ConvertArray(list.Values, element.Name, storage); - var listType = new ListType(new Field(element.Name, values.Data.DataType, element.IsNullable)); - return new ListArray(listType, list.Length, list.ValueOffsetsBuffer, values, list.NullBitmapBuffer, list.NullCount); - } + { + Field element = ((ListType)list.Data.DataType).ValueField; + IArrowArray values = ConvertArray(list.Values, element.Name, storage); + var listType = new ListType(new Field(element.Name, values.Data.DataType, element.IsNullable)); + return new ListArray(listType, list.Length, list.ValueOffsetsBuffer, values, list.NullBitmapBuffer, list.NullCount); + } default: return array; } From d85b12f29f3a92a83a4b53f43b7d388673d4011b Mon Sep 17 00:00:00 2001 From: Curt Hagenlocher Date: Sun, 27 Sep 2026 09:16:04 -0700 Subject: [PATCH 3/3] test: Cover Shred and Reassemble on large_binary and binary_view storage Co-Authored-By: Claude Opus 5.5 --- .../ShreddedVariantStorageTypeTests.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs b/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs index 9f268858..0843b783 100644 --- a/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs +++ b/test/Apache.Arrow.Operations.Tests/Shredding/ShreddedVariantStorageTypeTests.cs @@ -110,6 +110,48 @@ public void TypedAccessors_ReadLargeTypedColumns() Assert.Equal(new byte[] { 1, 2, 3 }, bin.GetBinaryBytes().ToArray()); } + [Theory] + [InlineData(Storage.LargeBinary, true)] + [InlineData(Storage.LargeBinary, false)] + [InlineData(Storage.BinaryView, true)] + [InlineData(Storage.BinaryView, false)] + public void Shred_ReadsAlternateStorage(Storage storage, bool shreddedInput) + { + VariantArray input = shreddedInput ? BuildShredded() : BuildUnshredded(); + + AssertRows(Convert(input, storage).Shred(Schema)); + } + + [Theory] + [InlineData(Storage.LargeBinary)] + [InlineData(Storage.BinaryView)] + public void Reassemble_ReadsAlternateStorage(Storage storage) + { + VariantArray reassembled = Convert(BuildShredded(), storage).Reassemble(); + + Assert.False(reassembled.IsShredded); + AssertRows(reassembled); + } + + private static VariantArray BuildUnshredded() + { + var builder = new VariantArray.Builder(); + builder.AppendRange(Rows); + return builder.Build(); + } + + private static void AssertRows(VariantArray array) + { + Assert.Equal(Rows.Count, array.Length); + for (int i = 0; i < Rows.Count; i++) + { + if (Rows[i].HasValue) + Assert.Equal(Rows[i].Value, array.GetLogicalVariantValue(i)); + else + Assert.True(array.IsNull(i)); + } + } + // --------------------------------------------------------------- // Storage conversion // ---------------------------------------------------------------