Skip to content

Commit

Permalink
fix(postgres): max number of binds is 65535, not 32767 (regression)
Browse files Browse the repository at this point in the history
  • Loading branch information
abonander committed Aug 26, 2024
1 parent 371cf4a commit 20ba796
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 6 deletions.
7 changes: 6 additions & 1 deletion sqlx-postgres/src/connection/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
17 changes: 14 additions & 3 deletions sqlx-postgres/src/message/bind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ use crate::message::{FrontendMessage, FrontendMessageFormat};
use crate::PgValueFormat;
use std::num::Saturating;

/// <https://www.postgresql.org/docs/current/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-BIND>
///
/// ## 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).
Expand All @@ -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],
Expand Down Expand Up @@ -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())
})?;

Expand All @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions sqlx-postgres/src/message/parameter_description.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ impl BackendMessage for ParameterDescription {
const FORMAT: BackendMessageFormat = BackendMessageFormat::ParameterDescription;

fn decode_body(mut buf: Bytes) -> Result<Self, Error> {
// 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);

Expand Down
4 changes: 3 additions & 1 deletion sqlx-postgres/src/message/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
56 changes: 55 additions & 1 deletion tests/postgres/query_builder.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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::<Postgres>().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, <i32 as Type<Postgres>>::type_info())
}
}
Either::Right(num_params) => {
assert_eq!(num_params, 65535);
}
}

let values: Vec<i32> = 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(())
}

0 comments on commit 20ba796

Please sign in to comment.