From 82b66b29a368b4d9b2c8082f2f3f27d5542aec8d Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Thu, 10 Dec 2020 22:02:01 +0300 Subject: [PATCH 1/3] Revert "Fixing BigDecimal conversion for PostgreSQL" This reverts commit a0007b4e98acf683069e360400742ee6934968cc. --- sqlx-core/src/postgres/types/bigdecimal.rs | 77 +++++++++++----------- tests/postgres/types.rs | 7 -- 2 files changed, 39 insertions(+), 45 deletions(-) diff --git a/sqlx-core/src/postgres/types/bigdecimal.rs b/sqlx-core/src/postgres/types/bigdecimal.rs index 617c19c5fe..28b2603c0a 100644 --- a/sqlx-core/src/postgres/types/bigdecimal.rs +++ b/sqlx-core/src/postgres/types/bigdecimal.rs @@ -1,7 +1,7 @@ use std::cmp; use std::convert::{TryFrom, TryInto}; -use bigdecimal::{BigDecimal, ToPrimitive, Zero}; +use bigdecimal::BigDecimal; use num_bigint::{BigInt, Sign}; use crate::decode::Decode; @@ -77,64 +77,65 @@ impl TryFrom<&'_ BigDecimal> for PgNumeric { type Error = BoxDynError; fn try_from(decimal: &BigDecimal) -> Result { - if decimal.is_zero() { - return Ok(PgNumeric::Number { - sign: PgNumericSign::Positive, - scale: 0, - weight: 0, - digits: vec![], - }); - } + let base_10_to_10000 = |chunk: &[u8]| chunk.iter().fold(0i16, |a, &d| a * 10 + d as i16); // NOTE: this unfortunately copies the BigInt internally let (integer, exp) = decimal.as_bigint_and_exponent(); + // this routine is specifically optimized for base-10 + // FIXME: is there a way to iterate over the digits to avoid the Vec allocation + let (sign, base_10) = integer.to_radix_be(10); + + // weight is positive power of 10000 + // exp is the negative power of 10 + let weight_10 = base_10.len() as i64 - exp; + // scale is only nonzero when we have fractional digits // since `exp` is the _negative_ decimal exponent, it tells us // exactly what our scale should be let scale: i16 = cmp::max(0, exp).try_into()?; - let (sign, uint) = integer.into_parts(); - let mut mantissa = uint.to_u128().unwrap(); + // there's an implicit +1 offset in the interpretation + let weight: i16 = if weight_10 <= 0 { + weight_10 / 4 - 1 + } else { + // the `-1` is a fix for an off by 1 error (4 digits should still be 0 weight) + (weight_10 - 1) / 4 + } + .try_into()?; - // If our scale is not a multiple of 4, we need to go to the next - // multiple. - let groups_diff = scale % 4; - if groups_diff > 0 { - let remainder = 4 - groups_diff as u32; - let power = 10u32.pow(remainder as u32) as u128; + let digits_len = if base_10.len() % 4 != 0 { + base_10.len() / 4 + 1 + } else { + base_10.len() / 4 + }; - mantissa = mantissa * power; - } + let offset = weight_10.rem_euclid(4) as usize; - // Array to store max mantissa of Decimal in Postgres decimal format. - let mut digits = Vec::with_capacity(8); + let mut digits = Vec::with_capacity(digits_len); - // Convert to base-10000. - while mantissa != 0 { - digits.push((mantissa % 10_000) as i16); - mantissa /= 10_000; + if let Some(first) = base_10.get(..offset) { + if offset != 0 { + digits.push(base_10_to_10000(first)); + } } - // Change the endianness. - digits.reverse(); - - // Weight is number of digits on the left side of the decimal. - let digits_after_decimal = (scale + 3) as u16 / 4; - let weight = digits.len() as i16 - digits_after_decimal as i16 - 1; + if let Some(rest) = base_10.get(offset..) { + digits.extend( + rest.chunks(4) + .map(|chunk| base_10_to_10000(chunk) * 10i16.pow(4 - chunk.len() as u32)), + ); + } - // Remove non-significant zeroes. while let Some(&0) = digits.last() { digits.pop(); } - let sign = match sign { - Sign::Plus | Sign::NoSign => PgNumericSign::Positive, - Sign::Minus => PgNumericSign::Negative, - }; - Ok(PgNumeric::Number { - sign, + sign: match sign { + Sign::Plus | Sign::NoSign => PgNumericSign::Positive, + Sign::Minus => PgNumericSign::Negative, + }, scale, weight, digits, diff --git a/tests/postgres/types.rs b/tests/postgres/types.rs index a0aa64eb69..93ac39761b 100644 --- a/tests/postgres/types.rs +++ b/tests/postgres/types.rs @@ -396,14 +396,7 @@ test_type!(bigdecimal(Postgres, "10000::numeric" == "10000".parse::().unwrap(), "0.1::numeric" == "0.1".parse::().unwrap(), "0.01::numeric" == "0.01".parse::().unwrap(), - "0.012::numeric" == "0.012".parse::().unwrap(), - "0.0123::numeric" == "0.0123".parse::().unwrap(), "0.01234::numeric" == "0.01234".parse::().unwrap(), - "0.012345::numeric" == "0.012345".parse::().unwrap(), - "0.0123456::numeric" == "0.0123456".parse::().unwrap(), - "0.01234567::numeric" == "0.01234567".parse::().unwrap(), - "0.012345678::numeric" == "0.012345678".parse::().unwrap(), - "0.0123456789::numeric" == "0.0123456789".parse::().unwrap(), "12.34::numeric" == "12.34".parse::().unwrap(), "12345.6789::numeric" == "12345.6789".parse::().unwrap(), )); From 9747218ad30a97eeb367c8a6d1a1e2d0b07af935 Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Thu, 10 Dec 2020 22:22:33 +0300 Subject: [PATCH 2/3] postgres: fix decimal conversions --- Cargo.lock | 37 ++++++++++++---------- sqlx-core/src/postgres/types/bigdecimal.rs | 32 ++++++++++++++++++- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 24f199fd9c..0b3871ac23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -40,6 +40,12 @@ dependencies = [ "threadpool", ] +[[package]] +name = "ahash" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6789e291be47ace86a60303502173d84af8327e3627ecf334356ee0f87a164c" + [[package]] name = "ahash" version = "0.5.8" @@ -1018,6 +1024,18 @@ name = "hashbrown" version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7afe4a420e3fe79967a00898cc1f4db7c8a49a9333a29f8a4bd76a253d5cd04" +dependencies = [ + "ahash 0.4.6", +] + +[[package]] +name = "hashlink" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d99cf782f0dc4372d26846bec3de7804ceb5df083c2d4462c0b8d2330e894fa8" +dependencies = [ + "hashbrown", +] [[package]] name = "heck" @@ -1197,12 +1215,6 @@ dependencies = [ "vcpkg", ] -[[package]] -name = "linked-hash-map" -version = "0.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8dd5a6d5999d9907cda8ed67bbd137d3af8085216c2ac62de5be860bd41f304a" - [[package]] name = "lock_api" version = "0.4.1" @@ -1221,15 +1233,6 @@ dependencies = [ "cfg-if 0.1.10", ] -[[package]] -name = "lru-cache" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "31e24f1ad8321ca0e8a1e0ac13f23cb668e6f5466c2c57319f6a5cf1cc8e3b1c" -dependencies = [ - "linked-hash-map", -] - [[package]] name = "maplit" version = "1.0.2" @@ -2200,7 +2203,7 @@ dependencies = [ name = "sqlx-core" version = "0.4.0" dependencies = [ - "ahash", + "ahash 0.5.8", "atoi", "base64 0.13.0", "bigdecimal", @@ -2220,6 +2223,7 @@ dependencies = [ "futures-core", "futures-util", "generic-array", + "hashlink", "hex", "hmac", "ipnetwork", @@ -2227,7 +2231,6 @@ dependencies = [ "libc", "libsqlite3-sys", "log", - "lru-cache", "md-5", "memchr", "num-bigint 0.3.1", diff --git a/sqlx-core/src/postgres/types/bigdecimal.rs b/sqlx-core/src/postgres/types/bigdecimal.rs index 28b2603c0a..7a3c5b016c 100644 --- a/sqlx-core/src/postgres/types/bigdecimal.rs +++ b/sqlx-core/src/postgres/types/bigdecimal.rs @@ -115,9 +115,11 @@ impl TryFrom<&'_ BigDecimal> for PgNumeric { let mut digits = Vec::with_capacity(digits_len); if let Some(first) = base_10.get(..offset) { - if offset != 0 { + if !first.is_empty() { digits.push(base_10_to_10000(first)); } + } else if offset != 0 { + digits.push(base_10_to_10000(&base_10) * 10i16.pow(3 - base_10.len() as u32)); } if let Some(rest) = base_10.get(offset..) { @@ -275,6 +277,34 @@ mod bigdecimal_to_pgnumeric { ); } + #[test] + fn one_hundredth() { + let one_hundredth: BigDecimal = "0.01".parse().unwrap(); + assert_eq!( + PgNumeric::try_from(&one_hundredth).unwrap(), + PgNumeric::Number { + sign: PgNumericSign::Positive, + scale: 2, + weight: -1, + digits: vec![100] + } + ); + } + + #[test] + fn twelve_thousandths() { + let twelve_thousandths: BigDecimal = "0.012".parse().unwrap(); + assert_eq!( + PgNumeric::try_from(&twelve_thousandths).unwrap(), + PgNumeric::Number { + sign: PgNumericSign::Positive, + scale: 3, + weight: -1, + digits: vec![120] + } + ); + } + #[test] fn decimal_1() { let decimal: BigDecimal = "1.2345".parse().unwrap(); From 35325ef6a10e371f2b6f5c3cc3a895cba062903f Mon Sep 17 00:00:00 2001 From: Chris Sosnin Date: Thu, 10 Dec 2020 22:30:51 +0300 Subject: [PATCH 3/3] Keep tests from reverted commit --- tests/postgres/types.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/postgres/types.rs b/tests/postgres/types.rs index 93ac39761b..a0aa64eb69 100644 --- a/tests/postgres/types.rs +++ b/tests/postgres/types.rs @@ -396,7 +396,14 @@ test_type!(bigdecimal(Postgres, "10000::numeric" == "10000".parse::().unwrap(), "0.1::numeric" == "0.1".parse::().unwrap(), "0.01::numeric" == "0.01".parse::().unwrap(), + "0.012::numeric" == "0.012".parse::().unwrap(), + "0.0123::numeric" == "0.0123".parse::().unwrap(), "0.01234::numeric" == "0.01234".parse::().unwrap(), + "0.012345::numeric" == "0.012345".parse::().unwrap(), + "0.0123456::numeric" == "0.0123456".parse::().unwrap(), + "0.01234567::numeric" == "0.01234567".parse::().unwrap(), + "0.012345678::numeric" == "0.012345678".parse::().unwrap(), + "0.0123456789::numeric" == "0.0123456789".parse::().unwrap(), "12.34::numeric" == "12.34".parse::().unwrap(), "12345.6789::numeric" == "12345.6789".parse::().unwrap(), ));