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

NUMERIC and DECIMAL support for Sqlite #2887

Closed
azam opened this issue Nov 20, 2023 · 7 comments · Fixed by #2890
Closed

NUMERIC and DECIMAL support for Sqlite #2887

azam opened this issue Nov 20, 2023 · 7 comments · Fixed by #2890
Labels
enhancement New feature or request

Comments

@azam
Copy link

azam commented Nov 20, 2023

Is your feature request related to a problem? Please describe.
I want to use the DECIMAL and NUMERIC data types on Sqlite, but sqlx-sqlite does not support it.

Describe the solution you'd like
Implementation of DECIMAL and NUMERIC on as supported types, with support for rust_decimal and bigdecimal

Describe alternatives you've considered
Using DOUBLE instead.
Using dual columns as INT to support scaled decimals instead.

Additional context
I have not found an issue or PR on the implementation, but please correct me if I am wrong.

@azam azam added the enhancement New feature or request label Nov 20, 2023
@dragonnn
Copy link
Contributor

Sqlx support for a db only those types that those db supports natively. As you might know sqlite doesn't have a type that you can map decimal too.

@azam
Copy link
Author

azam commented Nov 20, 2023

@dragonnn Thanks for the reply!

I don't see the rational in not supporting it, since sqlx already supports DATE and DATETIME which is also inferred (if I correctly get your meaning of native) based on the actual data type of the value.

https://github.com/launchbadge/sqlx/blob/main/sqlx-sqlite/src/type_info.rs#L50-L61

Ref: Sqlite Data Types - Affinity name examples
Sqlite Data Types - Affinity name examples

@abonander
Copy link
Collaborator

The problem is that while DATE and DATETIME have defined, precise representations in SQLite, there's no true support for arbitrary-precision decimals.

The SQLite documentation says the following of the NUMERIC type affinity: https://www.sqlite.org/datatype3.html

A column with NUMERIC affinity may contain values using all five storage classes. When text data is inserted into a NUMERIC column, the storage class of the text is converted to INTEGER or REAL (in order of preference) if the text is a well-formed integer or real literal, respectively. If the TEXT value is a well-formed integer literal that is too large to fit in a 64-bit signed integer, it is converted to REAL. For conversions between TEXT and REAL storage classes, only the first 15 significant decimal digits of the number are preserved. If the TEXT value is not a well-formed integer or real literal, then the value is stored as TEXT.
(Emphasis mine.)

My primary concern is that "well-formed real literal" is poorly defined, As written, it's distinctly possible that it will accept any decimal value, but truncate it to 15 significant digits and store it as a REAL (IEEE 754 float, effectively an f64).

Looking through the SQLite source, I think this interpretation is correct because it simply reads digits until reaching the sig-fig limit: https://github.com/sqlite/sqlite/blob/4f77a270325fe4a582f93f341516db5e2507b064/src/util.c#L543

It's a little weird because it uses a 64-bit unsigned integer as the significand, but it ultimately returns a double via an out-pointer.

We can also see pretty easily using the REPL that a value with a precision higher than 15 significant figures (20 in this case) is rounded off:

sqlite> SELECT CAST('1.2345678901234567890' as NUMERIC);
1.23456789012346
sqlite> SELECT TYPEOF(CAST('1.2345678901234567890' as NUMERIC));
real
sqlite> CREATE TABLE foo(bar numeric);
sqlite> INSERT INTO foo(bar) VALUES ('1.2345678901234567890');
sqlite> SELECT * FROM foo;
1.23456789012346

Since bigdecimal is arbitrary-precision and rust_decimal has a 96-bit mantissa, they both can produce values far outside the supported range, which may result in truncation and thus loss of precision, the exact opposite of what you usually want when reaching for these types.

I fear it would be really dangerous to provide direct mappings as users would be quick to utilize them without taking the time to properly understand their caveats, then open bugs when they encounter rounding errors. It would be a bad idea to have the macros choose these types automatically for the same reason.

Honestly, you probably just want to avoid the NUMERIC affinity like the plague and only use TEXT affinity.

I've been mulling around the idea of a Text<T> adapter that would encode and decode T using its Display and FromStr impls, respectively. That would allow use of these types (and many, many others) without us having to "officially" support the mapping.

@abonander
Copy link
Collaborator

The decimal.c extension seems like a good pairing, though if it's not included in the amalgamation then it's likely not part of the vendored source blob with libsqlite3-sys.

@azam
Copy link
Author

azam commented Nov 21, 2023

@abonander Thanks for the doc update and helping me understand the rational.
I would however liked having the support even though it might shoot the user in the foot.

@abonander
Copy link
Collaborator

I'll be working on a proof of concept for that Text adapter I mentioned, which would let you use any type you want. It would be pretty much equivalent anyway, as we'd just map rust_decimal::Decimal and BigDecimal to/from strings.

@azam
Copy link
Author

azam commented Nov 27, 2023

@abonander Thank would be nice. Saves a lot of writing FromRow impls :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants