From 791a7f5417ca46859ababd97ee0d52c0356f4024 Mon Sep 17 00:00:00 2001 From: Austin Bonander Date: Tue, 5 Mar 2024 18:06:41 -0800 Subject: [PATCH] doc(pg): document behavior of `bigdecimal` and `rust_decimal` with out-of-range values also add a regression test --- sqlx-postgres/src/types/bigdecimal-range.md | 20 ++++++++ sqlx-postgres/src/types/bigdecimal.rs | 26 +++++++--- sqlx-postgres/src/types/mod.rs | 4 ++ sqlx-postgres/src/types/rust_decimal-range.md | 10 ++++ sqlx-postgres/src/types/rust_decimal.rs | 6 +-- tests/postgres/postgres.rs | 48 +++++++++++++++++-- 6 files changed, 99 insertions(+), 15 deletions(-) create mode 100644 sqlx-postgres/src/types/bigdecimal-range.md create mode 100644 sqlx-postgres/src/types/rust_decimal-range.md diff --git a/sqlx-postgres/src/types/bigdecimal-range.md b/sqlx-postgres/src/types/bigdecimal-range.md new file mode 100644 index 00000000..5d4ee502 --- /dev/null +++ b/sqlx-postgres/src/types/bigdecimal-range.md @@ -0,0 +1,20 @@ +#### Note: `BigDecimal` Has a Larger Range than `NUMERIC` +`BigDecimal` can represent values with a far, far greater range than the `NUMERIC` type in Postgres can. + +`NUMERIC` is limited to 131,072 digits before the decimal point, and 16,384 digits after it. +See [Section 8.1, Numeric Types] of the Postgres manual for details. + +Meanwhile, `BigDecimal` can theoretically represent a value with an arbitrary number of decimal digits, albeit +with a maximum of 263 significant figures. + +Because encoding in the current API design _must_ be infallible, +when attempting to encode a `BigDecimal` that cannot fit in the wire representation of `NUMERIC`, +SQLx may instead encode a sentinel value that falls outside the allowed range but is still representable. + +This will cause the query to return a `DatabaseError` with code `22P03` (`invalid_binary_representation`) +and the error message `invalid scale in external "numeric" value` (though this may be subject to change). + +However, `BigDecimal` should be able to decode any `NUMERIC` value except `NaN`, +for which it has no representation. + +[Section 8.1, Numeric Types]: https://www.postgresql.org/docs/current/datatype-numeric.html diff --git a/sqlx-postgres/src/types/bigdecimal.rs b/sqlx-postgres/src/types/bigdecimal.rs index 9d66d359..f42a1794 100644 --- a/sqlx-postgres/src/types/bigdecimal.rs +++ b/sqlx-postgres/src/types/bigdecimal.rs @@ -127,10 +127,7 @@ impl TryFrom<&'_ BigDecimal> for PgNumeric { } Ok(PgNumeric::Number { - sign: match sign { - Sign::Plus | Sign::NoSign => PgNumericSign::Positive, - Sign::Minus => PgNumericSign::Negative, - }, + sign: sign_to_pg(sign), scale, weight, digits, @@ -138,17 +135,22 @@ impl TryFrom<&'_ BigDecimal> for PgNumeric { } } +#[doc=include_str!("bigdecimal-range.md")] impl Encode<'_, Postgres> for BigDecimal { fn encode_by_ref(&self, buf: &mut PgArgumentBuffer) -> IsNull { - use std::str::FromStr; // If the argument is too big, then we replace it with a less big argument. // This less big argument is already outside the range of allowed PostgreSQL DECIMAL, which // means that PostgreSQL will return the 22P03 error kind upon receiving it. This is the // expected error, and the user should be ready to handle it anyway. PgNumeric::try_from(self) .unwrap_or_else(|_| { - PgNumeric::try_from(&BigDecimal::from_str(&format!("{:030000}", 0)).unwrap()) - .unwrap() + PgNumeric::Number { + digits: vec![1], + // This is larger than the maximum allowed value, so Postgres should return an error. + scale: 0x4000, + weight: 0, + sign: sign_to_pg(self.sign()), + } }) .encode(buf); @@ -162,6 +164,9 @@ impl Encode<'_, Postgres> for BigDecimal { } } +/// ### Note: `NaN` +/// `BigDecimal` has a greater range than `NUMERIC` (see the corresponding `Encode` impl for details) +/// but cannot represent `NaN`, so decoding may return an error. impl Decode<'_, Postgres> for BigDecimal { fn decode(value: PgValueRef<'_>) -> Result { match value.format() { @@ -171,6 +176,13 @@ impl Decode<'_, Postgres> for BigDecimal { } } +fn sign_to_pg(sign: Sign) -> PgNumericSign { + match sign { + Sign::Plus | Sign::NoSign => PgNumericSign::Positive, + Sign::Minus => PgNumericSign::Negative, + } +} + #[cfg(test)] mod bigdecimal_to_pgnumeric { use super::{BigDecimal, PgNumeric, PgNumericSign}; diff --git a/sqlx-postgres/src/types/mod.rs b/sqlx-postgres/src/types/mod.rs index cf2c9ea9..d68d9b91 100644 --- a/sqlx-postgres/src/types/mod.rs +++ b/sqlx-postgres/src/types/mod.rs @@ -32,6 +32,8 @@ //! |---------------------------------------|------------------------------------------------------| //! | `bigdecimal::BigDecimal` | NUMERIC | //! +#![doc=include_str!("bigdecimal-range.md")] +//! //! ### [`rust_decimal`](https://crates.io/crates/rust_decimal) //! Requires the `rust_decimal` Cargo feature flag. //! @@ -39,6 +41,8 @@ //! |---------------------------------------|------------------------------------------------------| //! | `rust_decimal::Decimal` | NUMERIC | //! +#![doc=include_str!("rust_decimal-range.md")] +//! //! ### [`chrono`](https://crates.io/crates/chrono) //! //! Requires the `chrono` Cargo feature flag. diff --git a/sqlx-postgres/src/types/rust_decimal-range.md b/sqlx-postgres/src/types/rust_decimal-range.md new file mode 100644 index 00000000..f986d616 --- /dev/null +++ b/sqlx-postgres/src/types/rust_decimal-range.md @@ -0,0 +1,10 @@ +#### Note: `rust_decimal::Decimal` Has a Smaller Range than `NUMERIC` +`NUMERIC` is can have up to 131,072 digits before the decimal point, and 16,384 digits after it. +See [Section 8.1, Numeric Types] of the Postgres manual for details. + +However, `rust_decimal::Decimal` is limited to a maximum absolute magnitude of 296 - 1, +a number with 67 decimal digits, and a minimum absolute magnitude of 10-28, a number with, unsurprisingly, +28 decimal digits. + +Thus, in contrast with `BigDecimal`, `NUMERIC` can actually represent every possible value of `rust_decimal::Decimal`, +but not the other way around. This means that encoding should never fail, but decoding can. diff --git a/sqlx-postgres/src/types/rust_decimal.rs b/sqlx-postgres/src/types/rust_decimal.rs index 4a0c720f..9f00e3ca 100644 --- a/sqlx-postgres/src/types/rust_decimal.rs +++ b/sqlx-postgres/src/types/rust_decimal.rs @@ -71,6 +71,7 @@ impl TryFrom for Decimal { } } +// This impl is effectively infallible because `NUMERIC` has a greater range than `Decimal`. impl TryFrom<&'_ Decimal> for PgNumeric { type Error = BoxDynError; @@ -142,18 +143,17 @@ impl TryFrom<&'_ Decimal> for PgNumeric { } } -/// ### Panics -/// If this `Decimal` cannot be represented by `PgNumeric`. impl Encode<'_, Postgres> for Decimal { fn encode_by_ref(&self, buf: &mut PgArgumentBuffer) -> IsNull { PgNumeric::try_from(self) - .expect("Decimal magnitude too great for Postgres NUMERIC type") + .expect("BUG: `Decimal` to `PgNumeric` conversion should be infallible") .encode(buf); IsNull::No } } +#[doc=include_str!("rust_decimal-range.md")] impl Decode<'_, Postgres> for Decimal { fn decode(value: PgValueRef<'_>) -> Result { match value.format() { diff --git a/tests/postgres/postgres.rs b/tests/postgres/postgres.rs index 4f204e80..91728bde 100644 --- a/tests/postgres/postgres.rs +++ b/tests/postgres/postgres.rs @@ -933,11 +933,7 @@ from (values (null)) vals(val) #[sqlx_macros::test] async fn test_listener_cleanup() -> anyhow::Result<()> { - #[cfg(feature = "_rt-tokio")] - use tokio::time::timeout; - - #[cfg(feature = "_rt-async-std")] - use async_std::future::timeout; + use sqlx_core::rt::timeout; use sqlx::pool::PoolOptions; use sqlx::postgres::PgListener; @@ -1838,3 +1834,45 @@ async fn test_error_handling_with_deferred_constraints() -> anyhow::Result<()> { Ok(()) } + +#[sqlx_macros::test] +#[cfg(feature = "bigdecimal")] +async fn test_issue_3052() { + use sqlx::types::BigDecimal; + + // https://github.com/launchbadge/sqlx/issues/3052 + // Previously, attempting to bind a `BigDecimal` would panic if the value was out of range. + // Now, we rewrite it to a sentinel value so that Postgres will return a range error. + let too_small: BigDecimal = "1E-65536".parse().unwrap(); + let too_large: BigDecimal = "1E262144".parse().unwrap(); + + let mut conn = new::().await.unwrap(); + + let too_small_res = sqlx::query_scalar::<_, BigDecimal>("SELECT $1::numeric") + .bind(&too_small) + .fetch_one(&mut conn) + .await; + + match too_small_res { + Err(sqlx::Error::Database(dbe)) => { + let dbe = dbe.downcast::(); + + assert_eq!(dbe.code(), "22P03"); + } + other => panic!("expected Err(DatabaseError), got {other:?}"), + } + + let too_large_res = sqlx::query_scalar::<_, BigDecimal>("SELECT $1::numeric") + .bind(&too_large) + .fetch_one(&mut conn) + .await; + + match too_large_res { + Err(sqlx::Error::Database(dbe)) => { + let dbe = dbe.downcast::(); + + assert_eq!(dbe.code(), "22P03"); + } + other => panic!("expected Err(DatabaseError), got {other:?}"), + } +}