mongodb/specifications

Improper link in Decimal128 page

shanehsu opened this issue · 7 comments

The page currently links a specification of this, where this method is explained.

However, after implementing that, I found out the BSON C source actually implements this

And the comment in the driver seems to suggest this also. I only found this out after my implementation failed the test cases in test-decimal128.c.

Or am I mistaken?

I don't think the link is wrong. The spec does three things:

  • Provide a brief introduction to IEEE Decimal128, with a link to more detail
  • Specify how to serialize it to BSON using binary integer decimal format (not DPD)
  • Provides a large set of test cases

I'm in the middle finishing a bson library for the new Nim programming language and this one really threw me for a loop. I had eventually deduced the answer and while the docs don't mention that DPD is not being used, the regular reference to IEEE 754 2008 leads to docs that largely imply that DPD is the only correct way to do this. I actually wrote fully functioning sliding 10-bit DBD code and was frustrated by how the the mongo test cases never worked.

Just like the previous author, I confirmed the distinction by reading the C algo. Unfortunately "binary integer decimal format" can sound like descriptive wording rather than a spec.

So, may I suggest that, in just a few places, it should explicitly say "BSON does not use Densely Packed Binary for the significand (coefficient), instead it is stored as a simple 110bit unsigned integer." It would have saved me many hours of work and much frustration. I suspect it will help others also.

Side question: can I assume that the "leading decimal" that is stored in the Combination field is never used (always zero)? I've got code to support it :), but I can't think of a circumstance that it would be needed if not using DPD.

I'm happy to do a PR if this sounds acceptable to you all.

The first sentence of the specification links to the homepage for the specification for the encoding:

https://github.com/mongodb/specifications/blob/master/source/bson-decimal128/decimal128.rst#bson-decimal128-implementation-details

Would it help if the link was changed to the specification page instead?

@behackett It’s been some time since I worked on this. I think @JohnAD would be able to help you with the improvements to documentation so future driver implementor won’t be confused! Thanks to all :)

I will send a PR in the next few days with a suggested change.

Thanks!

BTW, to answer my own question a few message back: "can I assume that the "leading decimal" that is stored in the Combination field is never used (always zero)?". It is used because 34 nines requires a 113 bit number, not 110 bits. But, because of the structure of the combo field, you can ignore the state flags and parse the last 113 bits to get the decimals. The 114th bit can be safely ignored always for that particular calculation in BID. (Whereas with DPD, the 114th bit is needed if the leading digit is 8 or 9.)

Now that our docs clarify we do not use densely packed decimal format and #795 was merged I'm going to close this issue out.