Skip to content

Commit

Permalink
doc(pg): document behavior of bigdecimal and rust_decimal with ou…
Browse files Browse the repository at this point in the history
…t-of-range values

also add a regression test
  • Loading branch information
abonander committed Mar 6, 2024
1 parent e5c18b3 commit 791a7f5
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 15 deletions.
20 changes: 20 additions & 0 deletions sqlx-postgres/src/types/bigdecimal-range.md
Original file line number Diff line number Diff line change
@@ -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 2<sup>63</sup> 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
26 changes: 19 additions & 7 deletions sqlx-postgres/src/types/bigdecimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,28 +127,30 @@ 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,
})
}
}

#[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);

Expand All @@ -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<Self, BoxDynError> {
match value.format() {
Expand All @@ -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};
Expand Down
4 changes: 4 additions & 0 deletions sqlx-postgres/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,17 @@
//! |---------------------------------------|------------------------------------------------------|
//! | `bigdecimal::BigDecimal` | NUMERIC |
//!
#![doc=include_str!("bigdecimal-range.md")]
//!
//! ### [`rust_decimal`](https://crates.io/crates/rust_decimal)
//! Requires the `rust_decimal` Cargo feature flag.
//!
//! | Rust type | Postgres type(s) |
//! |---------------------------------------|------------------------------------------------------|
//! | `rust_decimal::Decimal` | NUMERIC |
//!
#![doc=include_str!("rust_decimal-range.md")]
//!
//! ### [`chrono`](https://crates.io/crates/chrono)
//!
//! Requires the `chrono` Cargo feature flag.
Expand Down
10 changes: 10 additions & 0 deletions sqlx-postgres/src/types/rust_decimal-range.md
Original file line number Diff line number Diff line change
@@ -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 2<sup>96</sup> - 1,
a number with 67 decimal digits, and a minimum absolute magnitude of 10<sup>-28</sup>, 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.
6 changes: 3 additions & 3 deletions sqlx-postgres/src/types/rust_decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ impl TryFrom<PgNumeric> for Decimal {
}
}

// This impl is effectively infallible because `NUMERIC` has a greater range than `Decimal`.
impl TryFrom<&'_ Decimal> for PgNumeric {
type Error = BoxDynError;

Expand Down Expand Up @@ -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<Self, BoxDynError> {
match value.format() {
Expand Down
48 changes: 43 additions & 5 deletions tests/postgres/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<Postgres>().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::<PgDatabaseError>();

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::<PgDatabaseError>();

assert_eq!(dbe.code(), "22P03");
}
other => panic!("expected Err(DatabaseError), got {other:?}"),
}
}

0 comments on commit 791a7f5

Please sign in to comment.