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

clean preparedStatements cache after successful deallocate #1795

Closed
wants to merge 1 commit into from

Conversation

Thiht
Copy link

@Thiht Thiht commented Nov 10, 2023

When trying to close a prepared statement on an aborted transaction, it's possible to clear the local prepared statement cache without actually deallocating the prepared statement in Postgres. So when trying to prepare the same statement a second time, this error occurs:

ERROR: prepared statement "stmt_xxx" already exists (SQLSTATE 42P05)

It's an issue we have in our tests but I wasn't able to isolate it as a runnable example (we use sqlx with the stdlib adapter, nested transactions, etc.) but the gist of it is:

  • start a transaction
  • prepare a statement stmt_xxx
  • defer Close/Deallocate it
  • abort the transaction (eg. SELECT 1/0)
  • the deallocate fails in DB but clears the preparedStatements cache
    • ERROR: current transaction is aborted, commands ignored until end of transaction block (SQLSTATE 25P02)
  • start a new transaction
  • prepare the same statement stmt_xxx
    • ERROR: prepared statement "stmt_xxx" already exists (SQLSTATE 42P05)

@Thiht Thiht changed the title clean ps cache after successful deallocate clean preparedStatements cache after successful deallocate Nov 10, 2023
@jackc
Copy link
Owner

jackc commented Nov 10, 2023

I think this would fail to deallocate statements where the name was automatically generated from the SQL string.

@Thiht
Copy link
Author

Thiht commented Nov 10, 2023

do you mean, like if you do something like this?

stmt, err := conn.Prepare(ctx, "select 1", "select 1")

?

in this case I'd say you're supposed to call Deallocate using stmt.Name, not the select 1 so I think it's not an issue, is it? or at least I don't think it changes the current behavior of Deallocate

@jackc
Copy link
Owner

jackc commented Nov 11, 2023

@Thiht After looking again I think you are right -- it didn't change behavior with deallocating statements with auto-generated names.

However, when I looked closer I think there is a deeper issue. Prepared statements are not transactional. That is, a prepared statement persists even if the transaction it was created it is rolled back. However, theDEALLOCATE statement would fail if run in an aborted transaction. But the PostgreSQL protocol has a protocol level means (that is, not a SQL statement) to deallocate statements -- and it works even in an aborted transaction. I've changed pgx to use that means in #1797. Can you test that and see if it solves the issue you discovered?

It also implements the behavior you suggest here, but it should be now impossible for Deallocate to fail unless the connection is entirely broken.

@Thiht
Copy link
Author

Thiht commented Nov 13, 2023

PostgreSQL protocol has a protocol level means (that is, not a SQL statement) to deallocate statements -- and it works even in an aborted transaction

Interesting stuff, seems like a good way to do in this case indeed.

I've changed pgx to use that means in #1797. Can you test that and see if it solves the issue you discovered?

Yep, I confirm it fixes the issue on my side.

I let you close this PR in favor of #1797 if everything's good for you

@jackc jackc closed this Nov 18, 2023
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.

2 participants