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

improve out of the box psycopg2 support #2273

Open
tychoish opened this issue Dec 17, 2023 · 2 comments
Open

improve out of the box psycopg2 support #2273

tychoish opened this issue Dec 17, 2023 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@tychoish
Copy link
Contributor

tychoish commented Dec 17, 2023

Presently, when attempting to connect to glaredb over the postgres protocol with the psycopg2 python module, unless connection.autocommit = True, psycopg2 will attempt to wrap all cursor operations in a transaction, sending BEGIN TRANSACTION queries to glaredb. As a result, queries throw an exception and users see the PGRES_EMPTY_QUERY error. Setting autocommit to true, disables the "wrap in a transaction" behavior.

The solution to this is somewhat difficult. We could (and should) improve the error message so users know that they were sending a transaction when they didn't mean to. It is (probably) correct to error early when we see a transaction that we shouldn't.

Uncovered by the testing in #2258.

@tychoish tychoish added the bug Something isn't working label Dec 17, 2023
@tychoish tychoish changed the title improve out of the box psycopg2 improve out of the box psycopg2 support Dec 17, 2023
@universalmind303
Copy link
Contributor

related to #2402

@tychoish
Copy link
Contributor Author

@scsmithr and I discussed this today and we decided:

  • we should document that the postgres protocol only does not support transactions.
  • glaredb should send a notice on the pg protocol when someone uses a trasaction statement, but otherwise continue.
  • cloud should print notices via the postgres protocol
  • in local mode, and for RPC/flight sql, transaction statements should be errors.

Some further carve outs.

  • we may also want to say that we may eventually provide transactions over the postgres protocol in the future.
  • it may make sense to still reject write operations inside of transactions.
  • it might make sense to reject some higher transaction levels too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants