shuttle-hq / synth

The Declarative Data Generator
https://www.getsynth.com/
Apache License 2.0
1.39k stars 109 forks source link

unable to insert {f32,f64} values into postgresql `decimal(m, n)` columns #220

Open vlushn opened 3 years ago

vlushn commented 3 years ago

Describe the bug

When inserting into (currently only tested with PostgreSQL) column that defines a custom decimal precision, e.g. decimal(7, 2), generated floats should "fit" into the column when defined with an appropriate step.

For example, in the case of a (7, 2) column, a step size of 0.01 should generate values that can fit into the column. However, sometimes values are generated that do not fit into the step, which causes a cryptic "error returned from database: insufficient data left in message" error.

To Reproduce

Create a column in postgres with type decimal(7, 2). Use the following definition

{
 "type": "number",
      "range": {
        "low": 0.0,
        "high": 100.0,
        "step": 0.01
      },
      "subtype": "f32",
}

Proceed to insert into postgres database and get the error above.

Expected behavior

The generated value should be in the sequence of {..., 110.01, 110.02, 110.03, ...} rather than contain something like 110.0199999999

vlushn commented 3 years ago

Actually this might be invalid (as a bug in synth) - after rewriting the impl Distribution<$target> for StandardFloatRangeStep<$target> in graph/number.rs I didn't get any differerent results and am beginning to think this is just the usual floating-point imprecision shenanigans that is not handled in some lower layer (e.g. sqlx)

vlushn commented 3 years ago

After switching the column to real the issue disappears, so my current thinking is either the Value encoding needs to be improved for decimal() type columns or there is a bug in sqlx for the same

christos-h commented 3 years ago

I think that for numeric and decimal we should support https://github.com/paupino/rust-decimal as per https://docs.rs/sqlx/0.5.9/sqlx/postgres/types/index.html.

This does raise the point however that Synth should have arbitrary precision types that can be used as a catch-all for situations like this. Not sure if @brokad has an opinion on this.