From 20ba796b0d0b8547d2a6f00720294e2ad46700d7 Mon Sep 17 00:00:00 2001 From: Austin Bonander Date: Mon, 26 Aug 2024 15:54:59 -0700 Subject: [PATCH] fix(postgres): max number of binds is 65535, not 32767 (regression) --- sqlx-postgres/src/connection/executor.rs | 7 ++- sqlx-postgres/src/message/bind.rs | 17 +++++- .../src/message/parameter_description.rs | 2 + sqlx-postgres/src/message/parse.rs | 4 +- tests/postgres/query_builder.rs | 56 ++++++++++++++++++- 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/sqlx-postgres/src/connection/executor.rs b/sqlx-postgres/src/connection/executor.rs index d2f6bcddf..d24dc8376 100644 --- a/sqlx-postgres/src/connection/executor.rs +++ b/sqlx-postgres/src/connection/executor.rs @@ -204,7 +204,12 @@ impl PgConnection { let format = if let Some(mut arguments) = arguments { // Check this before we write anything to the stream. - let num_params = i16::try_from(arguments.len()).map_err(|_| { + // + // Note: Postgres actually interprets this value as unsigned, + // making the max number of parameters 65535, not 32767 + // https://github.com/launchbadge/sqlx/issues/3464 + // https://www.postgresql.org/docs/current/limits.html + let num_params = u16::try_from(arguments.len()).map_err(|_| { err_protocol!( "PgConnection::run(): too many arguments for query: {}", arguments.len() diff --git a/sqlx-postgres/src/message/bind.rs b/sqlx-postgres/src/message/bind.rs index 83631fea5..4f58bdf5e 100644 --- a/sqlx-postgres/src/message/bind.rs +++ b/sqlx-postgres/src/message/bind.rs @@ -3,6 +3,12 @@ use crate::message::{FrontendMessage, FrontendMessageFormat}; use crate::PgValueFormat; use std::num::Saturating; +/// +/// +/// ## Note: +/// +/// The integer values for number of bind parameters, number of parameter format codes, +/// and number of result format codes all are interpreted as *unsigned*! #[derive(Debug)] pub struct Bind<'a> { /// The ID of the destination portal (`PortalId::UNNAMED` selects the unnamed portal). @@ -18,10 +24,11 @@ pub struct Bind<'a> { /// parameters; or it can equal the actual number of parameters. pub formats: &'a [PgValueFormat], + // Note: interpreted as unsigned, as is `formats.len()` and `result_formats.len()` /// The number of parameters. /// /// May be different from `formats.len()` - pub num_params: i16, + pub num_params: u16, /// The value of each parameter, in the indicated format. pub params: &'a [u8], @@ -64,7 +71,11 @@ impl FrontendMessage for Bind<'_> { buf.put_statement_name(self.statement); - let formats_len = i16::try_from(self.formats.len()).map_err(|_| { + // NOTE: the integer values for the number of parameters and format codes in this message + // are all interpreted as *unsigned*! + // + // https://github.com/launchbadge/sqlx/issues/3464 + let formats_len = u16::try_from(self.formats.len()).map_err(|_| { err_protocol!("too many parameter format codes ({})", self.formats.len()) })?; @@ -78,7 +89,7 @@ impl FrontendMessage for Bind<'_> { buf.extend(self.params); - let result_formats_len = i16::try_from(self.formats.len()) + let result_formats_len = u16::try_from(self.formats.len()) .map_err(|_| err_protocol!("too many result format codes ({})", self.formats.len()))?; buf.extend(result_formats_len.to_be_bytes()); diff --git a/sqlx-postgres/src/message/parameter_description.rs b/sqlx-postgres/src/message/parameter_description.rs index 8aa361a8e..f0b25ccc3 100644 --- a/sqlx-postgres/src/message/parameter_description.rs +++ b/sqlx-postgres/src/message/parameter_description.rs @@ -14,6 +14,8 @@ impl BackendMessage for ParameterDescription { const FORMAT: BackendMessageFormat = BackendMessageFormat::ParameterDescription; fn decode_body(mut buf: Bytes) -> Result { + // Note: this is correct, max parameters is 65535, not 32767 + // https://github.com/launchbadge/sqlx/issues/3464 let cnt = buf.get_u16(); let mut types = SmallVec::with_capacity(cnt as usize); diff --git a/sqlx-postgres/src/message/parse.rs b/sqlx-postgres/src/message/parse.rs index 3e77c3024..75300c481 100644 --- a/sqlx-postgres/src/message/parse.rs +++ b/sqlx-postgres/src/message/parse.rs @@ -43,7 +43,9 @@ impl FrontendMessage for Parse<'_> { buf.put_str_nul(self.query); - let param_types_len = i16::try_from(self.param_types.len()).map_err(|_| { + // Note: actually interpreted as unsigned + // https://github.com/launchbadge/sqlx/issues/3464 + let param_types_len = u16::try_from(self.param_types.len()).map_err(|_| { err_protocol!( "param_types.len() too large for binary protocol: {}", self.param_types.len() diff --git a/tests/postgres/query_builder.rs b/tests/postgres/query_builder.rs index 9c8c02d7b..08ed7d11a 100644 --- a/tests/postgres/query_builder.rs +++ b/tests/postgres/query_builder.rs @@ -1,6 +1,9 @@ use sqlx::postgres::Postgres; use sqlx::query_builder::QueryBuilder; -use sqlx::Execute; +use sqlx::Executor; +use sqlx::Type; +use sqlx::{Either, Execute}; +use sqlx_test::new; #[test] fn test_new() { @@ -103,3 +106,54 @@ fn test_query_builder_with_args() { "SELECT * FROM users WHERE id = $1 OR membership_level = $2" ); } + +#[sqlx::test] +async fn test_max_number_of_binds() -> anyhow::Result<()> { + // The maximum number of binds is 65535 (u16::MAX), not 32567 (i16::MAX) + // as the protocol documentation would imply + // + // https://github.com/launchbadge/sqlx/issues/3464 + + let mut qb: QueryBuilder<'_, Postgres> = QueryBuilder::new("SELECT ARRAY["); + + let mut elements = qb.separated(','); + + let max_bind = u16::MAX as i32; + + for i in 1..=max_bind { + elements.push_bind(i); + } + + qb.push("]::int4[]"); + + let mut conn = new::().await?; + + // Indirectly ensures the macros support this many binds since this is what they use. + let describe = conn.describe(qb.sql()).await?; + + match describe + .parameters + .expect("describe() returned no parameter information") + { + Either::Left(params) => { + assert_eq!(params.len(), 65535); + + for param in params { + assert_eq!(param, >::type_info()) + } + } + Either::Right(num_params) => { + assert_eq!(num_params, 65535); + } + } + + let values: Vec = qb.build_query_scalar().fetch_one(&mut conn).await?; + + assert_eq!(values.len(), 65535); + + for (idx, (i, j)) in (1..=max_bind).zip(values).enumerate() { + assert_eq!(i, j, "mismatch at index {idx}"); + } + + Ok(()) +}