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

Better handling of Numeric type #841

Closed
dbrgn opened this Issue Apr 5, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@dbrgn
Contributor

dbrgn commented Apr 5, 2017

Some operations like avg() on an integer field return Numeric, which isn't currently mapped to any Rust type. I didn't find out how to actually retrieve the resulting value. Is it even possible?

See #840.

@rubdos

This comment has been minimized.

Contributor

rubdos commented Apr 5, 2017

See PR #837. Would you be interested in a direct conversion to f64, and f32? If so, we should discuss whether that's a good idea or not; after all, you'd loose some data when doing the conversion, but for avg() it might not even be bad.

@dbrgn

This comment has been minimized.

Contributor

dbrgn commented Apr 5, 2017

@rubdos Numeric is something like a decimal type, right? so I guess what we'd lose is precision?

@sgrif

This comment has been minimized.

Member

sgrif commented Apr 5, 2017

I'd be fine with losing precision from avg and friends specifically, but I don't see any way to make that happen without allowing loss of precision for all cases which is not something I'd be willing to accept.

@dbrgn

This comment has been minimized.

Contributor

dbrgn commented Apr 5, 2017

If Numeric is a decimal type, maybe there's a loss-free decimal type available in the Rust ecosystem? Or maybe we could add our own wrapper that can be converted to a float explicitly, if desired?

@rubdos

This comment has been minimized.

Contributor

rubdos commented Apr 8, 2017

If Numeric is a decimal type, maybe there's a loss-free decimal type available in the Rust ecosystem? Or maybe we could add our own wrapper that can be converted to a float explicitly, if desired?

I refer again to #837; that directly solves this issue. The loss-free decimal type in the Rust ecosystem is done by the bigdecimal crate, by @akubera, @iterion and me, in collaboration with the num crate. The BigDecimal type allows for a direct conversion to float, which is currently in development.

@rubdos

This comment has been minimized.

Contributor

rubdos commented Apr 8, 2017

to_f64 is decently implemented now (before it only gave the integer value), and should be merged soon™.

Next up is finishing #837; I think all of what I need is available in BigDecimal or its merge requests now.

@killercup

This comment has been minimized.

Member

killercup commented Jun 25, 2017

Alright! #837 is on master. So, the next question is: What needs to happen to make this easily usable?

  • Numeric ↔ BigDecimal conversion
  • Full query/insert example of table using Numeric and struct using BigDecimal
  • Fix avg docs (and maybe look at other functions using Numeric)

Open points from #837:

  • Do we want FromSql<>/ToSql<> for f64/f32?
  • Support other databases; if someone wants to help here, feel free to file pull requests to my bigdecimal branch on my repo.

(Do we want to make this a meta issue for all things Numeric? I can move the todo lists to the main issue descriptions. Also, feel free to add more points, or open new issues for parts of this!)

@rubdos

This comment has been minimized.

Contributor

rubdos commented Jun 25, 2017

Do we want FromSql<>/ToSql<> for f64/f32?

Actually, I want to vote "no" on this. Although Rust does very well enforce types, I think this might confuse users. When you have Numeric in your database, you don't "just" want to read it out as f64/f32 as a test. Especially since now you can just go extern crate bigdecimal; struct A{a: BigDecimal} and be compatible with diesel...

(Do we want to make this a meta issue for all things Numeric? I can move the todo lists to the main issue descriptions. Also, feel free to add more points, or open new issues for parts of this!)

Good with me.

@rubdos

This comment has been minimized.

Contributor

rubdos commented Jun 25, 2017

Full query/insert example of table using Numeric and struct using BigDecimal

I grant you this and this file under both Apache 2.0 and MIT licenses; so you can use the code to put it in examples.

I don't have migrations in that project, but feel free to ask for more files if you want. There are example insertions, and example selections, and even some other fancy logic to make an invoice from them.

sgrif added a commit that referenced this issue Dec 16, 2017

Update the tests for `avg`
Right now there is no support for the `Numeric` type at all on SQLite
(since SQLite has no type that maps to it). This is a problem that we
need to fix later (probably by just treating it as an alias for
`Double` there).

The CFG attrs are really funky here. This is because tests for the main
crate run with multiple backends enabled, but the doctests need to run
against a single backend, and they have the equivalent to this:

    #[cfg(feature = "postgres")]
    type DB = Pg;

    #[cfg(all(not(feature = "postgres"), feature = "sqlite"))]
    type DB = Sqlite;

    #[cfg(all(not(any(feature = "postgres", feature = "sqlite")), feature = "mysql"))]
    type DB = Mysql;

In other words, the doctests run against the first of ["pg", "sqlite",
"mysql"] that is enabled, in that order... So this test needs to run if
pg is enabled (overrides SQLite) or SQLite is not enabled (so we must be
running on a backend that will pass).

Fixes #841 since this is really the last thing there that isn't fixed. I
will open a separate issue for the SQLite problem.

@sgrif sgrif closed this in #1408 Dec 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment