Skip to content

Fix #38 - support rust_decimal.#42

Merged
caibear merged 16 commits intomainfrom
38_rust_decimal
Aug 13, 2025
Merged

Fix #38 - support rust_decimal.#42
caibear merged 16 commits intomainfrom
38_rust_decimal

Conversation

@finnbear
Copy link
Copy Markdown
Member

Fixes #38

@finnbear
Copy link
Copy Markdown
Member Author

finnbear commented Sep 23, 2024

Remaining questions:

  • Is the Flags encoding maximally efficient?
    • justification of why it is: putting the sign bit at the LSB could help reduce the overall range of u8 values, allowing packing to work
  • Do all numbers round-trip properly?
    • need to check non-normalized decimal (with trailing zeroes)
    • strongly suspect -0 would be converted to 0 upon decoding (I want to fix this)

@SharksT
Copy link
Copy Markdown

SharksT commented May 14, 2025

Any update on this ? I would love to use this crate but I need rust decimal :)

@finnbear
Copy link
Copy Markdown
Member Author

Well, now there are two people who want it ;)

@l-7-l
Copy link
Copy Markdown

l-7-l commented Aug 12, 2025

@finnbear Maybe there are many, many people who want this. Perhaps precisely because their business now relies on rust_decimal, they currently cannot introduce bitcode.

Don't let it gather dust—merge it and let it shine! And also for uuid.

image

Thank you for this masterpiece

@finnbear finnbear requested a review from caibear August 12, 2025 03:19
@caibear
Copy link
Copy Markdown
Collaborator

caibear commented Aug 12, 2025

Before merging, I think we should

  • Avoid any unsafe code in the rust_decimal.rs file. It's not maintainable for us to add new unsafe code for each library that gets added.
  • Consider the Pros/Cons of this custom implementation vs a much simpler https://docs.rs/rust_decimal/latest/rust_decimal/struct.Decimal.html#method.serialize
  • These 2 ensure that the implementation doesn't use bitcode internals and could exist outside bitcode if ConvertFrom and impl_convert were exposed.

@caibear

This comment was marked as outdated.

@caibear caibear merged commit 3088cbd into main Aug 13, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for rust_decimal

4 participants