CSHARP-5792: Fix Decimal.MaxValue and Decimal.MinValue serialization…

[lukasvosyka]

Fix Decimal.MaxValue and Decimal.MinValue serialization behavior in RepresentationConverter, add decimal test cases to RepresentationConverterTests

[Copilot]

Pull request overview

This PR fixes the serialization behavior of decimal.MaxValue and decimal.MinValue when converting to and from Decimal128. The issue is that Decimal128.MaxValue/MinValue represent the maximum/minimum Decimal128 values (much larger than the decimal type can represent), while decimal.MaxValue/MinValue should map to new properties Decimal128.MaxDecimalValue/MinDecimalValue.

Key changes:

• Introduced Decimal128.MaxDecimalValue and Decimal128.MinDecimalValue properties to represent the System.Decimal bounds
• Updated RepresentationConverter.ToDecimal128() to return MaxDecimalValue/MinDecimalValue instead of MaxValue/MinValue for decimal extremes
• Updated RepresentationConverter.ToDecimal() to handle both old and new properties for backwards compatibility
• Added comprehensive test coverage for decimal<->Decimal128 conversions including edge cases

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/MongoDB.Bson/ObjectModel/Decimal128.cs	Added public properties MaxDecimalValue and MinDecimalValue to represent System.Decimal bounds as Decimal128 values
src/MongoDB.Bson/Serialization/Options/RepresentationConverter.cs	Fixed ToDecimal128 to return MaxDecimalValue/MinDecimalValue for decimal extremes; updated ToDecimal for backwards compatibility
tests/MongoDB.Bson.Tests/Serialization/Options/RepresentationConverterTests.cs	Added test cases covering decimal<->Decimal128 conversions with edge cases

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

[rstam]

I don't understand this.

We weren't comparing to Decimal128.MaxDecimalValue before, why now?

[rstam]

This seems wrong.

We should be mapping decimal.MaxValue to Decimal128.MaxValue, and we were!

Why the change?

[lukasvosyka]

I tried to find documentation on this behavior, because I don't understand why storing a completely different number in the database when it can store decimal.MaxValue. I would expect 79228162514264337593543950335 to be stored as "$numberDecimal" : "79228162514264337593543950335", but instead it gets stored as "$numberDecimal" : "9.999999999999999999999999999999999E+6144". As a side effect, when loading the document via BsonDocument and trying to get the value as decimal it would throw an error. So I filed CSHARP-5792.

Edit: Since you are saying

We should be mapping decimal.MaxValue to Decimal128.MaxValue, and we were!

I assume this is because other language driver implementations or what would the reason be? In this case my PR proposal doesn't make sense, indeed. Would it then make sense to add those checks to Decimal128 type at the start of public static decimal ToDecimal(Decimal128 d)? So the method would begin with this:

if (d == Decimal128.MaxValue)
{
    return decimal.MaxValue;
}
else if (d == Decimal128.MinValue)
{
    return decimal.MinValue;
}

In a nutshell, the following tests should be working, or?

[Fact]
public void Decimal128MaxValueShouldEqualDecimalMaxValue()
{
    Assert.Equal(decimal.MaxValue, (decimal)Decimal128.MaxValue);
}

[Fact]
public void DecimalMaxValueShouldEqualDecimal128MaxValue()
{
    Assert.Equal(Decimal128.MaxValue, (Decimal128)decimal.MaxValue);
}

[adelinowona]

@lukasvosyka Your edit is exactly right! The original mapping (decimal.MaxValue → Decimal128.MaxValue) is intentional and required for serialization consistency. Changing it now would be a behavioral breaking change. The bug is that Decimal128.ToDecimal() was missing the reverse mapping, causing the OverflowException you encountered. Please update your PR to keep the original RepresentationConverter.ToDecimal128() behavior.

[lukasvosyka]

@adelinowona I have reverted the changes I did to the RepresentationConverter and instead fixed Decimal128.ToDecimal(). I have left the additional tests in RepresentationConverterTests if you don't mind. It can serve as a "documentation" for this behavior in case someone else would have the same question. But if you don't like to have them there, I can also remove them again. Thanks all for taking time looking into this.