Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

graph, store: bump num-bigint and bigdecimal #2781

Closed

Conversation

vrmiguel
Copy link
Contributor

@vrmiguel vrmiguel commented Sep 8, 2021

This PR bumps

  • num-bigint from ^0.2.6 to ^0.4.7
  • bigdecimalfrom 0.1.0 to 0.3.0

This change has pros and cons, however.

Pros

Cons

  • The latest Diesel release still uses num-bigint 0.2.6 and bigdecimal 0.1.0.
    This means that we're no longer able to use the Diesel-defined ToSql<diesel::sql_types::Numeric, _> and FromSql<diesel::sql_types::Numeric, _> for BigDecimal implementations for version 0.3.0 of BigDecimal until Diesel upgrades their dependencies.
    In order to maintain support for these traits, I implemented the BigDecimalRetrocompatibility struct, which has functions for converting to and from versions 0.1 and 0.3 of BigDecimal.
    API limitations make this operation make heap allocations, although I believe those allocations should be small most of the time.

@tilacog @lutter @otaviopace @leoyvens
If you guys believe that this PR is the way to go, I'll add tests for BigDecimalRetrocompatibility and further docs on things I changed

@leoyvens
Copy link
Collaborator

Diesel 2.0 will upgrade these dependencies diesel-rs/diesel#2883 so we can choose to wait for that so we don't need the compatibility tricks.

@evaporei
Copy link
Contributor

@vrmiguel @leoyvens will this be continued? Otherwise I think we should close it.

@leoyvens
Copy link
Collaborator

Closing, we should upgrade this along with Diesel 2.0 which will be released soon.

@leoyvens leoyvens closed this Apr 25, 2022
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.

None yet

3 participants