Diesel: BigDecimal to_sql value has an issue

[lancecarlson]

Setup

Versions

• Rust:
  rustc 1.20.0-nightly (086eaa78e 2017-07-15)
• Diesel:
  master (as of ce82995bad7f3482b8b210f9784ea2d5a354da74)
• Database:
  postgres
• Operating System
  Fedora

Feature Flags

• diesel:
  postgres
• diesel_codegen:

Problem Description

I got this error when trying to parse this value: 0.000074. Not sure what the culprit is. I went back and stuck this in the test suit too:

---- types::pg_numeric_bigdecimal_to_sql stdout ----
    thread 'types::pg_numeric_bigdecimal_to_sql' panicked at 'slice index starts at 18446744073709551615 but ends at 1', /checkout/src/libcore/slice/mod.rs:741:4
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:390
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:497
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:92
   9: core::slice::slice_index_order_fail
             at /checkout/src/libcore/slice/mod.rs:741
  10: <core::ops::range::Range<usize> as core::slice::SliceIndex<[T]>>::index
             at /checkout/src/libcore/slice/mod.rs:864
  11: <core::ops::range::RangeFrom<usize> as core::slice::SliceIndex<[T]>>::index
             at /checkout/src/libcore/slice/mod.rs:947
  12: core::slice::<impl core::ops::index::Index<I> for [T]>::index
             at /checkout/src/libcore/slice/mod.rs:717
  13: <alloc::vec::Vec<T> as core::ops::index::Index<core::ops::range::RangeFrom<usize>>>::index
             at /checkout/src/liballoc/vec.rs:1593
  14: diesel::pg::types::numeric::bigdecimal::<impl core::convert::From<&'a bigdecimal::BigDecimal> for diesel::pg::types::floats::PgNumeric>::from
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/pg/types/numeric.rs:55
  15: diesel::pg::types::numeric::bigdecimal::<impl diesel::types::ToSql<diesel::types::Numeric, diesel::pg::backend::Pg> for bigdecimal::BigDecimal>::to_sql
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/pg/types/numeric.rs:87
  16: <diesel::query_builder::bind_collector::RawBytesBindCollector<DB> as diesel::query_builder::bind_collector::BindCollector<DB>>::push_bound_value
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_builder/bind_collector.rs:42
  17: <diesel::query_builder::ast_pass::AstPass<'a, DB>>::push_bind_param
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_builder/ast_pass.rs:87
  18: <diesel::expression::bound::Bound<T, U> as diesel::query_builder::QueryFragment<DB>>::walk_ast
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/expression/bound.rs:33
  19: <diesel::expression::operators::IsNull<Expr> as diesel::query_builder::QueryFragment<DB>>::walk_ast
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/expression/operators.rs:48
  20: <diesel::expression::operators::And<T, U> as diesel::query_builder::QueryFragment<DB>>::walk_ast
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/expression/operators.rs:48
  21: <diesel::expression::operators::Or<T, U> as diesel::query_builder::QueryFragment<DB>>::walk_ast
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/expression/operators.rs:48
  22: <diesel::expression::grouped::Grouped<T> as diesel::query_builder::QueryFragment<DB>>::walk_ast
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/expression/grouped.rs:16
  23: <diesel::query_builder::select_clause::SelectClause<T> as diesel::query_builder::select_clause::SelectClauseQueryFragment<QS, DB>>::walk_ast
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_builder/select_clause.rs:39
  24: <diesel::query_builder::select_statement::SelectStatement<(), S, D, W, O, L, Of, G> as diesel::query_builder::QueryFragment<DB>>::walk_ast
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_builder/select_statement/mod.rs:158
  25: diesel::query_builder::QueryFragment::collect_binds
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_builder/mod.rs:93
  26: diesel::pg::connection::PgConnection::prepare_query
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/pg/connection/mod.rs:109
  27: <diesel::pg::connection::PgConnection as diesel::connection::Connection>::query_by_index
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/pg/connection/mod.rs:75
  28: <T as diesel::query_dsl::load_dsl::LoadQuery<Conn, U>>::internal_load
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_dsl/load_dsl.rs:22
  29: diesel::query_dsl::load_dsl::LoadDsl::load
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_dsl/load_dsl.rs:33
  30: diesel::query_dsl::load_dsl::LoadDsl::get_result
             at /home/lcarlson/Projects/github.com/diesel-rs/diesel/diesel/src/query_dsl/load_dsl.rs:43
  31: integration_tests::types::query_to_sql_equality
             at tests/types.rs:788
  32: integration_tests::types::pg_numeric_bigdecimal_to_sql
             at tests/types.rs:468
  33: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1477
             at /checkout/src/libcore/ops/function.rs:143
             at /checkout/src/libtest/lib.rs:138
  34: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98

[killercup]

Thanks for the report! Let's try to get to the bottom of this!

This is the (at least) second bug found in our implementation of BigDecimal/interaction with the bigdecimal crate. Maybe it's a good idea to add a fuzzer for the roundtrip through bigdecimal, diesel, an postgres? (cc #907)

cc @rubdos

[lancecarlson]

I think that @eijebong was able to isolate the issue to values smaller than 1/10000. I haven't tested this yet but it seems right!

[rubdos]

Oh that 10k coding... Seems like we need tests sub 10^-4 and super 10^4 thus.

[Eijebong]

I opened #1096 to fix this one

[Eijebong]

Fixed by #1096

@killercup Should we open another issue asking to add more fuzzing to decimal tests ?

[killercup]

@Eijebong we already have #907. I'm not sure we need another but, but this is a concrete proposal, not an abstract idea… do whatever you think feels right :)

[Eijebong]

I forgot about this one... Well now they're linked, I think it's ok