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

fix: support queries with up to 65535 (inclusive) parameters #2525

Merged
merged 1 commit into from Jun 1, 2022

Conversation

vlsi
Copy link
Member

@vlsi vlsi commented May 31, 2022

Previously the execution failed with "Tried to send an out-of-range integer as a 2-byte value"
when the user attempted executing a query with more than 32767 parameters.

Technically speaking, the wire protocol limit is 2-byte-unsigned-int,
so we should support 65535 parameters.

In practice, simple mode (preferQueryMode=simple) allows executing queries
with an arbitrary number of parameters, however, that escape hatch is not recommended
as it still has limits on the SQL length, and it would likely be slow.

fixes #1311

@vlsi vlsi force-pushed the 32kbinds branch 2 times, most recently from ce40224 to 1fe3a38 Compare May 31, 2022 18:24
@vlsi
Copy link
Member Author

vlsi commented Jun 1, 2022

I just pushed revert commits to check if the CI duration improves.

In an unlikely case, "fix close refcursors" caused cursor leaks or something like that.

@vlsi
Copy link
Member Author

vlsi commented Jun 1, 2022

Rolling back "fix close refcursors" reduces CI duration from 3h50m to 50min.

@vlsi
Copy link
Member Author

vlsi commented Jun 1, 2022

Ok, the reason is "org.postgresql.test.jdbc2.RefCursorFetchTest |   | PASSED | 2h 57m 24.946s"

In other words, it does not look like a code regression, rather it looks like RefCursorFetchTest is extremely slow for unknown reasons.

@davecramer
Copy link
Member

Ok, the reason is "org.postgresql.test.jdbc2.RefCursorFetchTest |   | PASSED | 2h 57m 24.946s"

In other words, it does not look like a code regression, rather it looks like RefCursorFetchTest is extremely slow for unknown reasons.

that seems rather odd ?

@vlsi
Copy link
Member Author

vlsi commented Jun 1, 2022

It all means "fix close refcursors" was OK, and there's something else going slow with CI env.

The worst thing is that if I launch just org.postgresql.test.jdbc2.RefCursorFetchTest in Actions, then it takes ~16 or so seconds.

So it looks like some of the tests before RefCursorFetchTest triggers a bad condition, so subsequent tests become slow.

@davecramer
Copy link
Member

we can try to ask for support from github. They have been helpful in the past on security isssues.

@vlsi vlsi force-pushed the 32kbinds branch 4 times, most recently from ae847cd to 9c85185 Compare June 1, 2022 14:26
@vlsi
Copy link
Member Author

vlsi commented Jun 1, 2022

Finally, I nailed it: 82dbbe4#diff-dd8064c49349abfe7ba0b6e2d65ebb7d89600239c982dd0556bdace3765f751cL24-L25

SendRecvBufferSizeTest had System.setProperty("sendBufferSize", "1024"); System.setProperty("receiveBufferSize", "1024");

That caused all the test after SendRecvBufferSizeTest to use the small buffer size, which caused slow processing of large requests/responses.

Just in case you wonder, the trace log does show buffer sizes:

2022-05-31 20:10:33 FINE org.postgresql.Driver connect Connecting with URL: jdbc:postgresql://localhost:5432/test?ApplicationName=Driver Tests&loggerLevel=OFF&loggerFile=target/pgjdbc-tests.log
2022-05-31 20:10:33 FINE org.postgresql.jdbc.PgConnection <init> PostgreSQL JDBC Driver 42.3.7-SNAPSHOT
2022-05-31 20:10:33 FINE org.postgresql.jdbc.PgConnection setDefaultFetchSize   setDefaultFetchSize = 0
2022-05-31 20:10:33 FINE org.postgresql.jdbc.PgConnection setPrepareThreshold   setPrepareThreshold = 5
2022-05-31 20:10:33 FINE org.postgresql.core.v3.ConnectionFactoryImpl openConnectionImpl Trying to establish a protocol version 3 connection to localhost:5432
2022-05-31 20:10:33 FINE org.postgresql.core.v3.ConnectionFactoryImpl tryConnect Receive Buffer Size is 1,152
2022-05-31 20:10:33 FINE org.postgresql.core.v3.ConnectionFactoryImpl tryConnect Send Buffer Size is 2,304
2022-05-31 20:10:33 FINEST org.postgresql.core.v3.ConnectionFactoryImpl enableSSL  FE=> SSLRequest
2022-05-31 20:10:33 FINEST org.postgresql.core.v3.ConnectionFactoryImpl enableSSL  <=BE SSLOk
2022-05-31 20:10:33 FINE org.postgresql.ssl.MakeSSL convert converting regular socket connection to SSL

Previously the execution failed with "Tried to send an out-of-range integer as a 2-byte value"
when the user attempted executing a query with more than 32767 parameters.

Technically speaking, the wire protocol limit is 2-byte-unsigned-int,
so we should support 65535 parameters.

In practice, simple mode (preferQueryMode=simple) allows executing queries
with an arbitrary number of parameters, however, that escape hatch is not recommended
as it still has limits on the SQL length, and it would likely be slow.

fixes pgjdbc#1311
Copy link

@pwagland pwagland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message as it stands prints the wrong thing:

org.postgresql.util.PSQLException: PreparedStatement can have at most 65,535 parameters. Please consider using arrays, or splitting the query in several ones, or using COPY. Given query has 65,535 parameters
at org.postgresql.jdbc.PgPreparedStatement.<init>(PgPreparedStatement.java:102)
at org.postgresql.jdbc.PgPreparedStatement.<init>(PgPreparedStatement.java:88)

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

Successfully merging this pull request may close these issues.

How to resolve the "Tried to send an out-of-range integer as a 2-byte value" error
3 participants