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

`avg` cannot be used on SQLite #1409

Closed
sgrif opened this Issue Dec 16, 2017 · 0 comments

Comments

Projects
None yet
2 participants
@sgrif
Member

sgrif commented Dec 16, 2017

The return type of avg for all integer types is Numeric, which is the arbitrary precision floating point type. SQLite has no concept of this type. We can't really have the return type of avg be dependent on the backend (since we don't know the backend at the point avg is called). So we probably need to add some "support" for Numeric on SQLite. We should never infer this from diesel print-schema or anything like that, but we do need to do impl HasSqlType<Numeric> for Sqlite, and provide a FromSql impl for something there.

The question is whether we should have Numeric pretend to be Text, and provide a FromSql impl for BigDecimal, or whether we should have it pretend to be Double and provide an impl for f64. I'm guessing it should be the latter, since the floating point precision issues will happen on the DB side, but I'd like to see a query that demonstrates that avg(int_column) will in fact be limited by 64 bit float precision on SQLite regardless of what we do on our end.

@sgrif sgrif added this to the 1.2 milestone Jan 15, 2018

sgrif added a commit that referenced this issue Apr 5, 2018

Add partial support for `Numeric` on SQLite
Even though we can't support it as a proper type on that backend (there
is no native type to represent it), we still need this `FromSql` impl.
The way our query builder is structured, we can't vary the return type
of functions like `avg` based on the backend being used.

Even though this is subject to 64 bit precision, I opted to implement
support for `BigDecimal` instead of `f64` for consistency with the other
backends.

Fixes #1409.

sgrif added a commit that referenced this issue Apr 5, 2018

Add partial support for `Numeric` on SQLite
Even though we can't support it as a proper type on that backend (there
is no native type to represent it), we still need this `FromSql` impl.
The way our query builder is structured, we can't vary the return type
of functions like `avg` based on the backend being used.

Even though this is subject to 64 bit precision, I opted to implement
support for `BigDecimal` instead of `f64` for consistency with the other
backends.

Fixes #1409.

sgrif added a commit that referenced this issue Apr 6, 2018

Add partial support for `Numeric` on SQLite
Even though we can't support it as a proper type on that backend (there
is no native type to represent it), we still need this `FromSql` impl.
The way our query builder is structured, we can't vary the return type
of functions like `avg` based on the backend being used.

Even though this is subject to 64 bit precision, I opted to implement
support for `BigDecimal` instead of `f64` for consistency with the other
backends.

Fixes #1409.

@sgrif sgrif closed this in #1617 Apr 6, 2018

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