Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

self.param_types.len() <= (i16::MAX as usize) should change to u16::MAX #671

Open
sify21 opened this issue Sep 4, 2020 · 3 comments
Open

self.param_types.len() <= (i16::MAX as usize) should change to u16::MAX #671

sify21 opened this issue Sep 4, 2020 · 3 comments

Comments

@sify21
Copy link

@sify21 sify21 commented Sep 4, 2020

I'm using postgresql, sqlx="0.4.0-beta.1".
My struct is like:

#[derive(Serialize, Deserialize, Debug, PartialEq, FromRow)]
pub struct Struct1 {
    pub a1: i16,
    pub a2: String,
    pub a3: Option<String>,
    pub a4: Option<serde_json::Value>,
    pub a5: serde_json::Value,
    pub a6: Option<String>,
    pub a7: Option<serde_json::Value>,
    pub a8: Option<f32>,
    pub a9: Option<f32>,
    pub a10: Option<f32>,
    pub a11: Option<String>,
    pub a12: Option<f32>,
    pub a13: Option<f32>,
    pub a14: Option<f32>,
    pub a15: Option<String>,
    pub a16: Option<String>,
    pub a17: Option<String>,
    pub a18: Vec<String>,
}

My query string is like:

insert into table (...fields) values ($1,....),... on conflict(a2) do update set a1=excluded.a1,a2=.... returning *

I'm using this method to get results:

sqlx::query_as().bind().fetch_all()

This assertion fails:

thread 'main' panicked at 'assertion failed: self.param_types.len() <= (i16::MAX as usize)', ~/.cargo/registry/src/github.com-1ecc6299db9ec823/sqlx-core-0.4.0-beta.1/src/postgres/message/parse.rs:30:13

what's the problem here?

@sify21
Copy link
Author

@sify21 sify21 commented Sep 4, 2020

I have another smaller struct using the same pattern like above which succeeds.
Is it the number of columns in the table?

#[derive(Serialize, Deserialize, Debug, PartialEq, FromRow)]
pub struct Struct2 {
    pub a1: i16,
    pub a2: String,
    pub a3: Vec<String>,
    pub a4: Value,
}

I'm transiting from diesel-rs. In diesel I need to enable a feature called 128-column-tables.

@sify21
Copy link
Author

@sify21 sify21 commented Sep 4, 2020

Oh, I debugged and found the problem. It's the PARSE command's parameter length being too long.
libpq restricts to 65535. So when I'm using diesel, I insert 3000 Struct1 at a time, which means 54000 parameters.
But why does sqlx use i16::MAX? Shouldn't it be u16::MAX?

@sify21 sify21 changed the title got error: assertion failed: self.param_types.len() <= (i16::MAX as usize) assertion: self.param_types.len() <= (i16::MAX as usize) should change to u16::MAX Sep 4, 2020
@sify21 sify21 changed the title assertion: self.param_types.len() <= (i16::MAX as usize) should change to u16::MAX self.param_types.len() <= (i16::MAX as usize) should change to u16::MAX Sep 4, 2020
@abonander
Copy link
Collaborator

@abonander abonander commented Sep 4, 2020

So this is actually due to ambiguous documentation. We're also not the only ones to make the same mistake. The Postgres JDBC driver appears to cap the parameter count at Short.MAX_VALUE which is the same as i16::MAX:

jOOQ/jOOQ#5701
https://github.com/pgjdbc/pgjdbc/blob/4a4e664a868421e4ea8283b685a8abdf4ec907ab/pgjdbc/src/main/java/org/postgresql/core/PGStream.java#L346

The Postgres protocol documentation specifies that the bind parameter count is an Int16 and that IntN is "An n-bit integer in network byte order (most significant byte first)." It doesn't specify whether that integer is signed or unsigned, and context doesn't help either because in the same message definition (Bind (F)), -1 is used as the sentinel value for NULL, suggesting a signed interpretation of the int.

https://www.postgresql.org/docs/current/protocol-message-types.html
https://www.postgresql.org/docs/current/protocol-message-formats.html

The functions in libpq that take lists of parameters use a plain int for the number of parameters, and doesn't explicitly document the maximum number of bind parameters.

https://www.postgresql.org/docs/current/libpq-exec.html

You have to dig into the implementation to find this assertion: https://github.com/postgres/postgres/blob/7559d8ebfa11d98728e816f6b655582ce41150f3/src/interfaces/libpq/fe-exec.c#L1304

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.