diff --git a/src/Apache.Arrow.Scalars/Variant/VariantEncodingHelper.cs b/src/Apache.Arrow.Scalars/Variant/VariantEncodingHelper.cs
index 6975bfbb..5e87c21d 100644
--- a/src/Apache.Arrow.Scalars/Variant/VariantEncodingHelper.cs
+++ b/src/Apache.Arrow.Scalars/Variant/VariantEncodingHelper.cs
@@ -104,8 +104,8 @@ public static byte MakeShortStringHeader(int length) =>
// Object value header
// ---------------------------------------------------------------
//
- // Bits 2-3: field_id_size - 1 (0-3 => 1-4 bytes)
- // Bits 4-5: field_offset_size - 1 (0-3 => 1-4 bytes)
+ // Bits 2-3: field_offset_size - 1 (0-3 => 1-4 bytes)
+ // Bits 4-5: field_id_size - 1 (0-3 => 1-4 bytes)
// Bit 6: is_large (0 = 1-byte num_fields, 1 = 4-byte num_fields)
// Bit 7: unused (must be 0)
@@ -118,8 +118,8 @@ public static byte MakeShortStringHeader(int length) =>
public static byte MakeObjectHeader(int fieldIdSize, int offsetSize, bool isLarge)
{
int valueHeader =
- ((fieldIdSize - 1) & 0x03) |
- (((offsetSize - 1) & 0x03) << 2) |
+ ((offsetSize - 1) & 0x03) |
+ (((fieldIdSize - 1) & 0x03) << 2) |
((isLarge ? 1 : 0) << 4);
return MakeValueHeader(VariantBasicType.Object, valueHeader);
}
@@ -130,8 +130,8 @@ public static byte MakeObjectHeader(int fieldIdSize, int offsetSize, bool isLarg
public static void ParseObjectHeader(byte header, out int fieldIdSize, out int offsetSize, out bool isLarge)
{
int valueHeader = GetValueHeader(header);
- fieldIdSize = (valueHeader & 0x03) + 1;
- offsetSize = ((valueHeader >> 2) & 0x03) + 1;
+ offsetSize = (valueHeader & 0x03) + 1;
+ fieldIdSize = ((valueHeader >> 2) & 0x03) + 1;
isLarge = ((valueHeader >> 4) & 0x01) != 0;
}
diff --git a/test/Apache.Arrow.Scalars.Tests/TestVectors.cs b/test/Apache.Arrow.Scalars.Tests/TestVectors.cs
index 3c81622d..70e7e145 100644
--- a/test/Apache.Arrow.Scalars.Tests/TestVectors.cs
+++ b/test/Apache.Arrow.Scalars.Tests/TestVectors.cs
@@ -325,6 +325,37 @@ internal static class TestVectors
(byte)'B', (byte)'o', (byte)'b',
};
+ ///
+ /// The same object as — {"age": 30, "name": "Bob"} —
+ /// but encoded with field_id_size=2 and offset_size=1.
+ ///
+ /// Every other object vector here uses field_id_size == offset_size, where swapping the
+ /// two header fields is indistinguishable. This one does not, so it detects a reader that
+ /// reads the two size fields from each other's bits: such a reader sees field_id_size=1
+ /// and offset_size=2 and walks the id and offset lists at the wrong widths.
+ ///
+ /// A wider-than-minimal field_id_size is legal; the spec requires readers to honor the
+ /// width declared in the header.
+ ///
+ public static ReadOnlySpan Object_Age30_Name_Bob_WideFieldIds => new byte[]
+ {
+ 0x12, // header: basic_type=Object(2), fid_size=2, off_size=1, is_large=false
+ // value_header = (fid_size-1) << 2 | (off_size-1) = 0b000100 = 4
+ // header byte = (4 << 2) | 2 = 0x12
+ 0x02, // num_fields = 2
+ 0x00, 0x00, // field_id[0] = 0 (=> "age"), 2 bytes little-endian
+ 0x01, 0x00, // field_id[1] = 1 (=> "name"), 2 bytes little-endian
+ 0x00, // offset[0] = 0
+ 0x02, // offset[1] = 2
+ 0x06, // end_offset = 6
+ // value 0: Int8 = 30
+ 0x0C, // primitive Int8 header
+ 0x1E, // 30
+ // value 1: short string "Bob"
+ 0x0D, // basic_type=ShortString(1), length=3 => (3 << 2) | 1 = 13
+ (byte)'B', (byte)'o', (byte)'b',
+ };
+
// =================================================================
// Array test vectors
// =================================================================
diff --git a/test/Apache.Arrow.Scalars.Tests/VariantEncodingHelperTests.cs b/test/Apache.Arrow.Scalars.Tests/VariantEncodingHelperTests.cs
index f95bbf26..83589d9c 100644
--- a/test/Apache.Arrow.Scalars.Tests/VariantEncodingHelperTests.cs
+++ b/test/Apache.Arrow.Scalars.Tests/VariantEncodingHelperTests.cs
@@ -97,6 +97,45 @@ public void MakeAndParseObjectHeader(int fieldIdSize, int offsetSize, bool isLar
Assert.Equal(isLarge, parsedIsLarge);
}
+ // The round-trip test above cannot detect the field-id and offset size bits being
+ // swapped, because MakeObjectHeader and ParseObjectHeader would share the mistake. These
+ // two pin each direction to literal bytes taken from the spec's layout instead:
+ //
+ // value_header bits 0-1 = field_offset_size - 1
+ // value_header bits 2-3 = field_id_size - 1
+ // value_header bit 4 = is_large
+ // header byte = (value_header << 2) | Object(2)
+
+ [Theory]
+ [InlineData(1, 1, false, 0x02)]
+ [InlineData(2, 1, false, 0x12)]
+ [InlineData(1, 2, false, 0x06)]
+ [InlineData(2, 3, false, 0x1A)]
+ [InlineData(4, 1, true, 0x72)]
+ [InlineData(1, 4, true, 0x4E)]
+ [InlineData(4, 4, true, 0x7E)]
+ public void MakeObjectHeaderUsesSpecBitLayout(int fieldIdSize, int offsetSize, bool isLarge, int expected)
+ {
+ byte header = VariantEncodingHelper.MakeObjectHeader(fieldIdSize, offsetSize, isLarge);
+ Assert.Equal(expected, (int)header);
+ }
+
+ [Theory]
+ [InlineData(0x02, 1, 1, false)]
+ [InlineData(0x12, 2, 1, false)]
+ [InlineData(0x06, 1, 2, false)]
+ [InlineData(0x1A, 2, 3, false)]
+ [InlineData(0x72, 4, 1, true)]
+ [InlineData(0x4E, 1, 4, true)]
+ [InlineData(0x7E, 4, 4, true)]
+ public void ParseObjectHeaderUsesSpecBitLayout(int header, int expectedFieldIdSize, int expectedOffsetSize, bool expectedIsLarge)
+ {
+ VariantEncodingHelper.ParseObjectHeader((byte)header, out int fieldIdSize, out int offsetSize, out bool isLarge);
+ Assert.Equal(expectedFieldIdSize, fieldIdSize);
+ Assert.Equal(expectedOffsetSize, offsetSize);
+ Assert.Equal(expectedIsLarge, isLarge);
+ }
+
// ---------------------------------------------------------------
// Array headers
// ---------------------------------------------------------------
diff --git a/test/Apache.Arrow.Scalars.Tests/VariantReaderObjectTests.cs b/test/Apache.Arrow.Scalars.Tests/VariantReaderObjectTests.cs
index 10d72ef9..37c82025 100644
--- a/test/Apache.Arrow.Scalars.Tests/VariantReaderObjectTests.cs
+++ b/test/Apache.Arrow.Scalars.Tests/VariantReaderObjectTests.cs
@@ -150,6 +150,53 @@ public void TwoFields_TryGetField_Both()
Assert.False(obj.TryGetField("email", out VariantReader _));
}
+ // ---------------------------------------------------------------
+ // Asymmetric header sizes
+ // ---------------------------------------------------------------
+ //
+ // Objects whose field_id_size and offset_size differ are the only ones that can detect
+ // the two header size fields being read from each other's bits. Every vector above uses
+ // field_id_size == offset_size, where the two layouts coincide.
+
+ [Fact]
+ public void WideFieldIds_FieldNames()
+ {
+ VariantObjectReader obj = new VariantObjectReader(
+ TestVectors.SortedMetadata_Age_Name, TestVectors.Object_Age30_Name_Bob_WideFieldIds);
+
+ Assert.Equal(2, obj.FieldCount);
+ Assert.Equal("age", obj.GetFieldName(0));
+ Assert.Equal("name", obj.GetFieldName(1));
+ }
+
+ [Fact]
+ public void WideFieldIds_FieldValues()
+ {
+ VariantObjectReader obj = new VariantObjectReader(
+ TestVectors.SortedMetadata_Age_Name, TestVectors.Object_Age30_Name_Bob_WideFieldIds);
+
+ VariantReader ageValue = obj.GetFieldValue(0);
+ Assert.Equal(VariantPrimitiveType.Int8, ageValue.PrimitiveType);
+ Assert.Equal(30, ageValue.GetInt8());
+
+ VariantReader nameValue = obj.GetFieldValue(1);
+ Assert.True(nameValue.IsString);
+ Assert.Equal("Bob", nameValue.GetString());
+ }
+
+ [Fact]
+ public void WideFieldIds_TryGetField_Both()
+ {
+ VariantObjectReader obj = new VariantObjectReader(
+ TestVectors.SortedMetadata_Age_Name, TestVectors.Object_Age30_Name_Bob_WideFieldIds);
+
+ Assert.True(obj.TryGetField("age", out VariantReader ageValue));
+ Assert.Equal(30, ageValue.GetInt8());
+
+ Assert.True(obj.TryGetField("name", out VariantReader nameValue));
+ Assert.Equal("Bob", nameValue.GetString());
+ }
+
// ---------------------------------------------------------------
// Error cases
// ---------------------------------------------------------------