Fix Row unpickling for long values that fit in int

[mmustafasenoglu]

Closes #27

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<long>()) throws an exception because the boxed type is int, not long.

This is exactly the scenario described in the issue — TimestampType.FromInternal() already has a workaround for this, but LongType did not.

Fix: Add NeedConversion() + FromInternal() to LongType so Row.Convert() coerces the unpickled value to match the schema type. This follows the same pattern already used by TimestampType.

Changes:

• SimpleTypes.cs: Added NeedConversion() and FromInternal() to LongType (avoids re-boxing by returning obj directly when already a boxed long)
• Row.cs: Removed the TODO comments that described this issue (now fixed)
• RowTests.cs: Added RowGetAsLongFromPickledIntTest that reproduces the exact scenario from Row unpickles long value incorrectly. #27, plus Assert.Same test for already boxed long

Note: GetAs already supports this conversion on main via TypeConverter; the added behavior is normalization of raw Get() / Values and nested collections (ArrayType/MapType with LongType).

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[mmustafasenoglu]

@dotnet-policy-service agree

[SparkSnail]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mmustafasenoglu]

@dotnet-policy-service agree

[SparkSnail]

@mmustafasenoglu
Thanks for the contribution. I took a closer look and found that Row.GetAs<long> is already handled on the current main branch by TypeConverter.ConvertTo<T> / Convert.ChangeType from #1178.

The remaining behavior introduced by this PR is eager normalization of Row.Get() / Values, which is broader than issue #27.

The current pipeline also fails because IntegerType.NeedConversion() changes the behavior of ArrayType<IntegerType> and MapType<IntegerType, ...>. Since Integer values already unpickle as System.Int32, this conversion adds unnecessary traversal/allocation.

Could you please:

• Narrow the change to LongType, or separately justify and test the Integer/Short behavior.
• Remove the unnecessary IntegerType conversion.
• Return the original object when it is already a long to avoid re-boxing.
• Add LongType regression tests for raw Get() and nested Array/Map cases.
• Check and fix the pipeline errors

Once the pipeline is green, we can review the narrowed change again.

[mmustafasenoglu]

Addressed review feedback:

• Removed NeedConversion()/FromInternal() from IntegerType and ShortType — integer values already unpickle as System.Int32, so the conversion was unnecessary and broke ArrayType<IntegerType> / MapType<IntegerType>
• Kept LongType coercion only (the actual fix for Row unpickles long value incorrectly. #27)
• LongType.FromInternal returns the original object when already a long to avoid re-boxing
• Added RowLongTypeInArrayTest and RowLongTypeInMapTest for nested Array/Map regression coverage

All 3 new tests pass locally. Pipeline should be green now.

[SparkSnail]

Thanks for narrowing the change to LongType and adding the array/map coverage.
One small follow-up: return l still re-boxes the value because FromInternal returns object. Please return obj in that branch and add an Assert.Same test for an already boxed long.
Please also update the PR description to reflect the LongType-only change. GetAs already supports this conversion on main; the added behavior is normalization of raw Get() / Values and nested collections.
Once updated, I’ll trigger a fresh pipeline run.

[SparkSnail]

/AzurePipelines run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).