Copilot commented on code in PR #332:
URL: https://github.com/apache/arrow-dotnet/pull/332#discussion_r3143669691
##########
src/Apache.Arrow.Scalars/Variant/VariantValue.cs:
##########
@@ -113,6 +113,24 @@ public static VariantValue FromDecimal8(decimal value) =>
public static VariantValue FromDecimal16(decimal value) =>
new VariantValue(VariantPrimitiveType.Decimal16, (object)value);
+ /// <summary>
+ /// Creates a Decimal16 variant value from a <see cref="SqlDecimal"/>,
always
+ /// producing <see cref="VariantPrimitiveType.Decimal16"/>. Values
exceeding
+ /// <see cref="decimal"/> range are stored as <see cref="SqlDecimal"/>.
+ /// Use this when the target type is known (e.g. materializing a
Decimal16
+ /// shredded column); use <see cref="FromSqlDecimal(SqlDecimal)"/>
when you
+ /// want the smallest decimal type that fits the value.
+ /// </summary>
+ public static VariantValue FromDecimal16(SqlDecimal value)
+ {
+ if (value.Data[3] != 0)
+ {
+ SqlDecimal normalized = SqlDecimal.ConvertToPrecScale(value,
38, value.Scale);
+ return new VariantValue(VariantPrimitiveType.Decimal16,
(object)normalized);
+ }
+ return new VariantValue(VariantPrimitiveType.Decimal16,
(object)value.Value);
Review Comment:
FromDecimal16(SqlDecimal) converts to decimal via value.Value when
value.Data[3] == 0. SqlDecimal.Value can still throw for values that aren't
representable as System.Decimal (e.g., scale/precision beyond decimal’s limits)
even when the magnitude fits in 96 bits. Consider storing the SqlDecimal
instance in those cases (or using a try/catch fallback) so Decimal16
materialization can’t unexpectedly overflow.
```suggestion
try
{
return new VariantValue(VariantPrimitiveType.Decimal16,
(object)value.Value);
}
catch (OverflowException)
{
SqlDecimal normalized = SqlDecimal.ConvertToPrecScale(value,
38, value.Scale);
return new VariantValue(VariantPrimitiveType.Decimal16,
(object)normalized);
}
```
--
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]
