Address the issues uncovered by running Npgsql tests #788
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #593, Jelte notified us that running Npgsql test suite against PgCat's prepared statements results in some failures.
Running the tests uncovered the following issues
Handling
DISCARD ALL
/DEALLOCATE ALL
The current implementation of Prepared statements in transactions does not handle these two commands correctly. When they arrive, we pass them down to the underlying server connection and that results in a desynchronization between the prepared statements that PgCat thinks are on the server connection and what is actually there. So when it gets a request to execute a prepared statement that it thinks is on the server, it passes it down and that results in query failure with error
prepared statement XX doesn't exist
The fix is to reset PgCat prepared statement cache for the connection when we get these calls.
Handling cases where client expects a
Bind Complete
response but gets aParse Complete
When the client sends a
Parse
statement to create a named prepared statement, it expects that it can sendBind
messages and they would work.Under transaction mode, the client is likely to checkout server connections that don't have the prepared statement. The way PgCat handles this is to send a
Parse
statement under-the-hood and then pass theBind
with some changes to the naming scheme.When we send
Parse
thenBind
, we get backParse Complete
andBind Complete
from the server. Now, we currently forward both messages to the client. This is not the accurate response because the client only sent aBind
so it expects to get aBind Complete
notParse Complete
followed byBind Complete
The Npgsql test suite is pretty strict and it catches the edge case and fails the test. Most clients seem to be okay with this.
Fixing this will require some big refactor to keep track of what messages we shouldn't pass down to the client. I won't pursue a fix for this issue in this PR
Misinterpreting the the number of parameters on Parse for very large parameter list
This one has to do with
Parse
messageWe use the
Int16
value to know how big of a vector we need to allocate, however, we read that value in ani16
notu16
so while the limit on the number of parameters should be 65k, we would get a negative number if we get more than32767
parameters and plugging that parameter into an allocation operation results in a panic. Kudos to Npgsql team for testing this edge case.