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

SqlMagic.autocommit=True is not applied to connection options #90

Closed
yafimvo opened this issue Jan 26, 2023 · 7 comments · Fixed by #91
Closed

SqlMagic.autocommit=True is not applied to connection options #90

yafimvo opened this issue Jan 26, 2023 · 7 comments · Fixed by #91
Assignees
Labels
bug Something isn't working

Comments

@yafimvo
Copy link

yafimvo commented Jan 26, 2023

Running

%load_ext sql
%config SqlMagic.autocommit=True
%sql postgresql://localhost/
%sql CREATE DATABASE new_db

returns the following error:

InternalError: (psycopg2.errors.ActiveSqlTransaction) CREATE DATABASE cannot run inside a transaction block
@yafimvo yafimvo added the bug Something isn't working label Jan 26, 2023
@yafimvo yafimvo self-assigned this Jan 26, 2023
@edublancas
Copy link

was this executed in a new session? The error suggests that the CREATE DATABASE statement was executed inside the middle of a transaction

@yafimvo
Copy link
Author

yafimvo commented Jan 26, 2023

@edublancas it happens when executing the session here.

According to docs, setting the autocommit option should fix it but according to the implementation, setting SqlMagic.autocommit doesn't affect the connection settings.

I didn't reference the original issue since I'm not sure we want to support this feature and if SqlMagic.autocommit=True and conn.autocommit = True are really equal.

@idomic
Copy link

idomic commented Jan 26, 2023

Reference

@edublancas
Copy link

I looked at the code to understand why this is happening.

It turns out, ipython-sql isn't using the psycopg2's autocommit feature. %config SqlMagic.autocommit=True sets a flag that causes ipython-sql to run COMMIT after each command; so, if we turn it on, running this:

CREATE DATABASE my_test_db

is the same as running this:

BEGIN; -- executed since psycopg2's autocommit is off
CREATE DATABASE my_test_db; -- executed by the user
COMMIT; -- executed by ipython-sql due to the autocommit=True

Which causes the error:

CREATE DATABASE cannot run inside a transaction block


Psycopg2 issues the initial BEGIN command since the native autocommit flag is off (do not confuse it with our config.autocommit flag), as documented in a warning here.

It's best to rely on the driver's autocommit feature rather than manually issuing a COMMIT command. However, since this is driver-dependent, we should only do it selectively. Perhaps something like this:

if config.autocommit:
    try:
        conn.session.execution_options(isolation_level="AUTOCOMMIT")
    except Exception as e:
        # show some warning
        # commit manually
        manual_commit = True
    else:
        manual_commit = False

Then, use the manual_commit flag to issue a COMMIT command after running each query.

@edublancas
Copy link

edublancas commented Feb 10, 2023

since we closed #117, now we can test this.

the first thing is to check what happens when running:

%load_ext sql
%config SqlMagic.autocommit=True
%sql postgresql://localhost/
%sql CREATE DATABASE new_db

we already know that postgres fails, but we'll see what happens with the others.

then, we must check what happens with @yafimvo's solution: #91

my guess is that the final solution will involve some checking logic as I described in my earlier comment.

since you worked on the integration tests, do you want to take this @tonykploomber ?

@tonykploomber
Copy link

I can take this

@idomic
Copy link

idomic commented Mar 14, 2023

FYI @jameshowison , this will be on the next release!

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

Successfully merging a pull request may close this issue.

4 participants