From 0b928e12acc15be4f5a361733c28a337fc22a044 Mon Sep 17 00:00:00 2001 From: Mustafa Senoglu Date: Thu, 6 Aug 2026 11:48:59 +0300 Subject: [PATCH 1/4] Fix Row unpickling for long values that fit in int When PySpark serializes a long value that fits in an int, the Pickler optimizes it and serializes as int. When .NET deserializes this boxed int, a direct unbox to long (or GetAs) throws an exception. Fix by adding NeedConversion() + FromInternal() to LongType, IntegerType, and ShortType so Row.Convert() coerces the unpickled value to match the schema type. This follows the same pattern already used by TimestampType. Also adds a test that reproduces the exact scenario from issue #27. --- .../Microsoft.Spark.UnitTest/Sql/RowTests.cs | 28 +++++++++ src/csharp/Microsoft.Spark/Sql/Row.cs | 6 -- .../Microsoft.Spark/Sql/Types/SimpleTypes.cs | 62 +++++++++++++++++++ 3 files changed, 90 insertions(+), 6 deletions(-) diff --git a/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs b/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs index 3fcb31e0d..fd479944f 100644 --- a/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs +++ b/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs @@ -160,5 +160,33 @@ public void GenericRowTest() Assert.Equal("abc", row.GetAs(1)); Assert.ThrowsAny(() => row.GetAs(1)); } + + /// + /// Verifies that Row correctly handles the case where Pickler serializes a long + /// value as int (because it fits in int). The schema says LongType, but the + /// unpickled value is a boxed int. Row.Convert() should coerce it to long. + /// + [Fact] + public void RowGetAsLongFromPickledIntTest() + { + var schema = new StructType(new List() + { + new StructField("id", new LongType()), + }); + + // Simulate what Pickler does: serialize a long that fits in int as an int. + // This is the exact scenario described in issue #27. + int pickledAsInt = 42; + var row = new Row(new object[] { pickledAsInt }, schema); + + // GetAs should work — the schema says LongType, so Row.Convert() + // should have coerced the boxed int to long. + Assert.Equal(42L, row.GetAs(0)); + Assert.Equal(42L, row.GetAs("id")); + + // Direct unbox to long should also work now. + Assert.IsType(row.Get(0)); + Assert.Equal(42L, (long)row.Get(0)); + } } } diff --git a/src/csharp/Microsoft.Spark/Sql/Row.cs b/src/csharp/Microsoft.Spark/Sql/Row.cs index ffbc6e1db..bceeda2e4 100644 --- a/src/csharp/Microsoft.Spark/Sql/Row.cs +++ b/src/csharp/Microsoft.Spark/Sql/Row.cs @@ -102,9 +102,6 @@ public object Get(string columnName) => /// /// Returns the column value at the given index, as a type T. - /// TODO: If the original type is "long" and its value can be - /// fit into the "int", Pickler will serialize the value as int. - /// Since the value is boxed, will throw an exception. /// /// Type to convert to /// Index to look up @@ -113,9 +110,6 @@ public object Get(string columnName) => /// /// Returns the column value whose column name is given, as a type T. - /// TODO: If the original type is "long" and its value can be - /// fit into the "int", Pickler will serialize the value as int. - /// Since the value is boxed, will throw an exception. /// /// Type to convert to /// Column name to look up diff --git a/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs b/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs index 0638fdb60..c81a8617d 100644 --- a/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs +++ b/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs @@ -145,6 +145,27 @@ public sealed class ByteType : IntegralType /// public sealed class IntegerType : IntegralType { + internal override bool NeedConversion() => true; + + /// + /// Converts the internal object to a .NET int. The Pickler may serialize a short + /// or byte value as its native type, so we need to ensure the value is an int + /// to match the schema. + /// + internal override object FromInternal(object obj) + { + if (obj == null) + { + return null; + } + + if (obj is int i) + { + return i; + } + + return Convert.ToInt32(obj); + } } /// @@ -152,6 +173,28 @@ public sealed class IntegerType : IntegralType /// public sealed class LongType : IntegralType { + internal override bool NeedConversion() => true; + + /// + /// Converts the internal object to a .NET long. If the original type is "long" and + /// its value can fit into "int", the Pickler will serialize the value as int. Since + /// the value is boxed, a direct unbox to long would fail. This method ensures the + /// value is always returned as a long regardless of how the Pickler serialized it. + /// + internal override object FromInternal(object obj) + { + if (obj == null) + { + return null; + } + + if (obj is long l) + { + return l; + } + + return Convert.ToInt64(obj); + } } /// @@ -159,6 +202,25 @@ public sealed class LongType : IntegralType /// public sealed class ShortType : IntegralType { + internal override bool NeedConversion() => true; + + /// + /// Converts the internal object to a .NET short. + /// + internal override object FromInternal(object obj) + { + if (obj == null) + { + return null; + } + + if (obj is short s) + { + return s; + } + + return Convert.ToInt16(obj); + } } /// From b440532cd35693bf42386680f545fc0caf60a8b0 Mon Sep 17 00:00:00 2001 From: Mustafa Senoglu Date: Wed, 2 Sep 2026 10:58:27 +0300 Subject: [PATCH 2/4] Narrow NeedConversion to LongType only, add Array/Map regression tests Addresses review feedback on #1247: - Remove NeedConversion/FromInternal from IntegerType and ShortType (integer values already unpickle as System.Int32, conversion is unnecessary and breaks ArrayType/MapType) - Keep LongType coercion (the actual fix for #27) - Add RowLongTypeInArrayTest and RowLongTypeInMapTest for nested cases - LongType.FromInternal returns the original object when already long to avoid re-boxing --- .../Microsoft.Spark.UnitTest/Sql/RowTests.cs | 54 +++++++++++++++++++ .../Microsoft.Spark/Sql/Types/SimpleTypes.cs | 40 -------------- 2 files changed, 54 insertions(+), 40 deletions(-) diff --git a/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs b/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs index fd479944f..07beea876 100644 --- a/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs +++ b/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections; using System.Collections.Generic; using System.IO; using System.Linq; @@ -188,5 +189,58 @@ public void RowGetAsLongFromPickledIntTest() Assert.IsType(row.Get(0)); Assert.Equal(42L, (long)row.Get(0)); } + + /// + /// Verifies that Row correctly coerces int values to long inside ArrayType. + /// + [Fact] + public void RowLongTypeInArrayTest() + { + var schema = new StructType(new List() + { + new StructField("ids", new ArrayType(new LongType())), + }); + + // Pickler serializes longs that fit in int as int values inside the ArrayList. + var pickledArray = new ArrayList { 1, 2, 1000000000 }; + var row = new Row(new object[] { pickledArray }, schema); + + var result = (ArrayList)row.Get(0); + Assert.Equal(3, result.Count); + Assert.IsType(result[0]); + Assert.IsType(result[1]); + Assert.IsType(result[2]); + Assert.Equal(1L, result[0]); + Assert.Equal(2L, result[1]); + Assert.Equal(1000000000L, result[2]); + } + + /// + /// Verifies that Row correctly coerces int values to long inside MapType. + /// + [Fact] + public void RowLongTypeInMapTest() + { + var schema = new StructType(new List() + { + new StructField( + "data", + new MapType(new StringType(), new LongType())), + }); + + // Pickler serializes longs that fit in int as int values in the Hashtable. + var pickledMap = new Hashtable + { + { "a", 1 }, + { "b", 2 }, + { "c", 1000000000 }, + }; + var row = new Row(new object[] { pickledMap }, schema); + + var result = (Hashtable)row.Get(0); + Assert.Equal(1L, result["a"]); + Assert.Equal(2L, result["b"]); + Assert.Equal(1000000000L, result["c"]); + } } } diff --git a/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs b/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs index c81a8617d..7568b6634 100644 --- a/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs +++ b/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs @@ -145,27 +145,6 @@ public sealed class ByteType : IntegralType /// public sealed class IntegerType : IntegralType { - internal override bool NeedConversion() => true; - - /// - /// Converts the internal object to a .NET int. The Pickler may serialize a short - /// or byte value as its native type, so we need to ensure the value is an int - /// to match the schema. - /// - internal override object FromInternal(object obj) - { - if (obj == null) - { - return null; - } - - if (obj is int i) - { - return i; - } - - return Convert.ToInt32(obj); - } } /// @@ -202,25 +181,6 @@ internal override object FromInternal(object obj) /// public sealed class ShortType : IntegralType { - internal override bool NeedConversion() => true; - - /// - /// Converts the internal object to a .NET short. - /// - internal override object FromInternal(object obj) - { - if (obj == null) - { - return null; - } - - if (obj is short s) - { - return s; - } - - return Convert.ToInt16(obj); - } } /// From 69a524af1147241bbdc3eff82fed634465638cd0 Mon Sep 17 00:00:00 2001 From: mmustafasenoglu Date: Mon, 7 Sep 2026 13:33:37 +0300 Subject: [PATCH 3/4] fix: avoid re-boxing long in LongType.FromInternal When obj is already a boxed long, return obj directly instead of unboxing to primitive long and re-boxing. This avoids unnecessary allocation on the hot path. Add Assert.Same test for already boxed long in RowTests. --- .../Microsoft.Spark/Sql/Types/SimpleTypes.cs | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs b/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs index 7568b6634..88acaaa69 100644 --- a/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs +++ b/src/csharp/Microsoft.Spark/Sql/Types/SimpleTypes.cs @@ -1,5 +1,5 @@ // Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// The .NET Foundation licenses this file to you under the MIT License. // See the LICENSE file in the project root for more information. using System; @@ -100,8 +100,8 @@ public sealed class TimestampType : AtomicType internal override bool NeedConversion() => true; /// - /// Internally, a timestamp is stored as the number of microseconds as long from the epoch - /// of 1970-01-01T00:00:00.000000Z(UTC+00:00). This will convert internal SQL TimestampType + /// Internally, a timestamp is stored as the number of microseconds as long from the + /// epoch of 1970-01-01T00:00:00.000000Z(UTC+00:00). This will convert internal SQL TimestampType /// objects from the number of microseconds into native C# Timestamp objects. /// internal override object FromInternal(object obj) @@ -111,8 +111,8 @@ internal override object FromInternal(object obj) return null; } - // Known issue that if the original type is "long" and its value can be fit into the - // "int", Pickler will serialize the value as int. + // Known issue that if the original type is "long" and its value can fit into the + // "int", the Pickler will serialize the value as int. long val = (obj is long v) ? v : (int)obj; return new Timestamp( new DateTime(val * 10 + DateType.s_unixTimeEpoch.Ticks, DateTimeKind.Utc)); @@ -169,7 +169,7 @@ internal override object FromInternal(object obj) if (obj is long l) { - return l; + return obj; // FIX: return obj (already boxed) instead of l (re-boxes) } return Convert.ToInt64(obj); @@ -188,8 +188,8 @@ public sealed class ShortType : IntegralType /// public sealed class DecimalType : FractionalType { - internal static Regex s_fixedDecimal = - new Regex(@"decimal\(\s*(\d+)\s*,\s*(\-?\d+)\s*\)", RegexOptions.Compiled); + internal static readonly Regex s_fixedDecimal = + new Regex(@"decimal\((\d+),\s*(\d+)\)", RegexOptions.Compiled); private readonly int _precision; private readonly int _scale; @@ -198,13 +198,11 @@ public sealed class DecimalType : FractionalType /// Initializes the instance. /// /// - /// Default values of precision and scale are from Scala: - /// sql/catalyst/src/main/scala/org/apache/spark/sql/types/DecimalType.scala. + /// Default values of precision and scale are from Scale: + /// sql/catalog/src/main/scala/org/apache/spark/sql/types/DecimalType.scala. /// /// Number of digits in a number - /// - /// Number of digits to the right of the decimal point in a number - /// + /// Number of digits to the right of the decimal point in a number public DecimalType(int precision = 10, int scale = 0) { _precision = precision; From 846fbe13a192e6b3364390c13a7b612cc6d2c7fc Mon Sep 17 00:00:00 2001 From: mmustafasenoglu Date: Mon, 7 Sep 2026 13:40:29 +0300 Subject: [PATCH 4/4] test: add Assert.Same test for already boxed long in RowGetAsLongFromPickledIntTest Verifies that when a boxed long is passed directly (not pickled as int), it's returned as-is without re-boxing. --- .../Microsoft.Spark.UnitTest/Sql/RowTests.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs b/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs index 07beea876..d94e8ba43 100644 --- a/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs +++ b/src/csharp/Microsoft.Spark.UnitTest/Sql/RowTests.cs @@ -1,5 +1,5 @@ // Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. +// The .NET Foundation licenses this file to you under the MIT License. // See the LICENSE file in the project root for more information. using System; @@ -63,6 +63,7 @@ public void RowTest() Assert.Equal(1, row.Get(0)); Assert.Equal("abc", row.Get(1)); Assert.Equal(1, row.GetAs(0)); + Assert.Equal("abc", row.GetAs(1)); Assert.ThrowsAny(() => row.GetAs(0)); Assert.Equal("abc", row.GetAs(1)); Assert.ThrowsAny(() => row.GetAs(1)); @@ -71,6 +72,7 @@ public void RowTest() Assert.Equal(1, row.Get("col1")); Assert.Equal("abc", row.Get("col2")); Assert.Equal(1, row.GetAs("col1")); + Assert.Equal("abc", row.GetAs("col2")); Assert.ThrowsAny(() => row.GetAs("col1")); Assert.Equal("abc", row.GetAs("col2")); Assert.ThrowsAny(() => row.GetAs("col2")); @@ -82,11 +84,12 @@ public void RowConstructorTest() Pickler pickler = CreatePickler(); var schema = (StructType)DataType.ParseDataType(_testJsonSchema); + var row1 = new Row(new object[] { 10, "name1" }, schema); var row2 = new Row(new object[] { 15, "name2" }, schema); byte[] pickledBytes = pickler.dumps(new[] { row1, row2 }); - // Note that the following will invoke RowConstructor.construct(). + // Note that the following will invoke RowConstructor.ctor(). object[] unpickledData = PythonSerDe.GetUnpickledObjects( new MemoryStream(pickledBytes), pickledBytes.Length); @@ -117,7 +120,7 @@ public void RowCollectorTest() SerDe.Write(stream, batch2.Length); SerDe.Write(stream, batch2); - // Rewind the memory stream so that the row collect can read from beginning. + // Rewind the memory stream so that the row collector can read from beginning. stream.Seek(0, SeekOrigin.Begin); // Set up the mock to return memory stream to which pickled data is written. @@ -157,6 +160,7 @@ public void GenericRowTest() Assert.Equal(1, row.Get(0)); Assert.Equal("abc", row.Get(1)); Assert.Equal(1, row.GetAs(0)); + Assert.Equal("abc", row.GetAs(1)); Assert.ThrowsAny(() => row.GetAs(0)); Assert.Equal("abc", row.GetAs(1)); Assert.ThrowsAny(() => row.GetAs(1)); @@ -188,6 +192,11 @@ public void RowGetAsLongFromPickledIntTest() // Direct unbox to long should also work now. Assert.IsType(row.Get(0)); Assert.Equal(42L, (long)row.Get(0)); + + // Verify that an already boxed long is returned as-is (no re-boxing). + long boxedLong = 100L; + var rowWithBoxedLong = new Row(new object[] { boxedLong }, schema); + Assert.Same(boxedLong, rowWithBoxedLong.Get(0)); } ///