mkleehammer / pyodbc

Python ODBC bridge
https://github.com/mkleehammer/pyodbc/wiki
MIT No Attribution
2.88k stars 561 forks source link

How to get the "xact_state" value of a transaction? #1295

Closed nishantvarma closed 8 months ago

nishantvarma commented 8 months ago

Environment

Issue

Is it possible to get the xact_state value of a transaction (to check if it's committable) with SQLAlchemy + pyODBC, when there is an exception? Refer https://github.com/sqlalchemy/sqlalchemy/discussions/7982 to see how to get that using try-catch block.

Quite simply put:

session.add(User)
session.flush() 
session.execute("select * from user").fetchall() --> returns results
try:
    session.execute("SELECT CAST('A' AS INT)").fetchall()[0][0]  --> transaction is rolled back
except:
    session.execute("select xact_state") --> Expected -1, Observed: 1

I want to write code such that if xact_state is -1, I just want to raise out. In all other cases, I want to continue the transaction.

try:
    session.execute("SELECT CAST('A' AS INT)").fetchall()  --> transaction is rolled back
except:
    if session.execute("select xact_state").fetchall()[0][0] == -1:
        raise
    else:
       logger.exception("This query failed. Continuing with the next statement.")

The only solution I have now is to check if "some known previous value" inserted during the transaction is the same; if not raise out. However, this is not a generic solution.

v-chojas commented 8 months ago

It's not clear what exactly you're asking. If you're using SQLAlchemy then ask there, unless you are saying there is an issue in pyODBC?

nishantvarma commented 8 months ago

@v-chojas , Sorry, didn't make it clear. I have asked this in SQLAlchemy (refer to the link above, not quite using the session.execute example though), but was redirected herer. I am trying to see if I can use pyODBC's connection object (as it's more closer to the DB) to achieve this. Something like:

session.connection().connection.execute("select xact_state")

PS: If you still think it's not a valid question, I'll close this. I just thought it belongs to pyODBC (as suggested by Michael himself - unless there is a miscommunication).

gordthompson commented 8 months ago

Why do you expect select xact_state() to return -1 (uncommittable)? Yes, the SELECT CAST(…) is invalid, but it does not roll back the transaction nor does it render the transaction uncommittable.

with Session(engine) as session:
    user_1 = session.get(User, 1)
    """
    BEGIN (implicit)
    SELECT [user].id AS user_id, [user].name AS user_name 
    FROM [user] 
    WHERE [user].id = ?
    [generated in 0.00038s] (1,)
    """
    user_1.name = "Gord"
    session.flush()
    """
    UPDATE [user] SET name=? WHERE [user].id = ?
    [generated in 0.00043s] ('Gord', 1)
    """
    try:
        session.execute(text("SELECT CAST('A' AS INT)")).fetchall()
    except:
        result = session.scalar(text("select xact_state()"))
        print(result)  # 1 (transaction in effect and can be committed)
    session.commit()
    """
    COMMIT
    """
gordthompson commented 8 months ago

Ahh, wait a minute. SQLAlchemy isn't rolling back the transaction but apparently something is.

If I do this, the update is applied as expected.

import pyodbc

cnxn = pyodbc.connect(
    "Driver=ODBC Driver 17 for SQL Server;"
    "Server=192.168.0.199;"
    "UID=scott;PWD=tiger^5HHH;"
    "Database=test;",
    autocommit=False,
)
crsr = cnxn.cursor()

crsr.execute("DROP TABLE IF EXISTS [user]")
crsr.execute("CREATE TABLE [user] (id int primary key, name nvarchar(50))")
crsr.execute("INSERT INTO [user] (id, name) VALUES (1, N'Before')")
cnxn.commit()

crsr.execute("UPDATE [user] SET name = N'After' WHERE id = 1")
cnxn.commit()

print(crsr.execute("SELECT * FROM [user]").fetchall())
# [(1, 'After')]

However, if I add the try/except block, the update does not "stick"

import pyodbc

cnxn = pyodbc.connect(
    "Driver=ODBC Driver 17 for SQL Server;"
    "Server=192.168.0.199;"
    "UID=scott;PWD=tiger^5HHH;"
    "Database=test;",
    autocommit=False,
)
crsr = cnxn.cursor()

crsr.execute("DROP TABLE IF EXISTS [user]")
crsr.execute("CREATE TABLE [user] (id int primary key, name nvarchar(50))")
crsr.execute("INSERT INTO [user] (id, name) VALUES (1, N'Before')")
cnxn.commit()

crsr.execute("UPDATE [user] SET name = N'After' WHERE id = 1")
try:
    crsr.execute("SELECT CAST('A' AS INT)")
except:
    result = crsr.execute("SELECT xact_state()").fetchval()
    print(result)  # 1
cnxn.commit()

print(crsr.execute("SELECT * FROM [user]").fetchall())
# [(1, 'Before')]

odbctrace.log

nishantvarma commented 8 months ago

Yeah, that's the problem I am facing. The insert/update doesn't "stick". I know the root cause of this problem: any "Level 16" errors aborts the transaction; but I am unable to "detect" this in SQLAlchemy. SQLAlchemy's session apparently doesn't understand this, causing HUGE problems in my application (thousands of man-hours wasted) that does lot of these try-excepts (I believe this is why they always ask to rollback).

This works (i.e. such queries won't cause a silent rollback in try-excepts):

BEGIN TRANSACTION
  BEGIN TRY
    SELECT 1/0;
  END TRY
  BEGIN CATCH
    PRINT XACT_STATE();
  END CATCH
COMMIT

This fails (i.e. such queries cause a silent rollback in try-excepts):

BEGIN TRANSACTION
  BEGIN TRY
    SELECT CAST('A' AS INT);
  END TRY
  BEGIN CATCH
    PRINT XACT_STATE();
  END CATCH
COMMIT

I am trying to detect this in SQLAlchemy (or pyODBC) so that I can handle it. I have a dirty hack of checking if some value that should should get updated is still intact; but that's not a generic solution.

gordthompson commented 8 months ago

So are the values of XACT_STATE() different in the above two cases?

nishantvarma commented 8 months ago

Yes, the XACT_STATE of SELECT 1/0 is 1; but SELECT CAST('A' AS INT) is -1. I think MSSQL considers such (runtime) errors dangerous and aborts the transaction. Perhaps this is why setting SET XACT_ABORT ON is considered as a good practice.

gordthompson commented 8 months ago

Maybe there isn't a way to get the "real" values via ODBC. I see the same values for XACT_STATE() and @@TRANCOUNT in both cases:

crsr.execute("SET XACT_ABORT ON")
crsr.execute("UPDATE [user] SET name = N'After' WHERE id = 1")
try:
    crsr.execute("SELECT CAST('A' AS INT)")
except:
    print(crsr.execute("SELECT XACT_STATE()").fetchval())  # 1
    print(crsr.execute("SELECT @@TRANCOUNT").fetchval())  # 1
cnxn.commit()

print(crsr.execute("SELECT * FROM [user]").fetchall())
# [(1, 'Before')]
crsr.execute("SET XACT_ABORT ON")
crsr.execute("UPDATE [user] SET name = N'After' WHERE id = 1")
try:
    crsr.execute("SELECT 1/0)")
except:
    print(crsr.execute("SELECT XACT_STATE()").fetchval())  # 1
    print(crsr.execute("SELECT @@TRANCOUNT").fetchval())  # 1
cnxn.commit()

print(crsr.execute("SELECT * FROM [user]").fetchall())
# [(1, 'After')]
nishantvarma commented 8 months ago

Thank You for your valuable time, @gordthompson. If that's the case, I'll use some "proxy" values (update queries issued in beginning) to check if the transaction is still alive. It seemed like a simple problem :-)

session.execute("update activity set status="inprogress")

try:
     session.execute(...)
except:
    if session.execute("select status from activity").fetchall()[0][0] != "inprogress":
        raise

The problem is that it's not a generic solution.

gordthompson commented 8 months ago

Okay, so pyodbc can't retrieve the xact_state() value correctly, but pymssql can. Details at https://github.com/sqlalchemy/sqlalchemy/discussions/7982#discussioncomment-7382066

nishantvarma commented 8 months ago

Thanks, @gordthompson . I will keep this information in mind (don't think switching drivers will be an option). Hopefully, ODBC brings support for this; it seems like an important information to have. I will see if I can report it upstream as well. I am closing this issue as it is not a pyodbc issue.

zzzeek commented 8 months ago

@v-chojas , Sorry, didn't make it clear. I have asked this in SQLAlchemy (refer to the link above; not quite using the session.execute example though), but was redirected to pyODBC.

I want to clarify that I also said this should be done using the pyodbc cursor directly - it's natural for the pyodbc people to be skeptical when they see a bunch of SQLAlchemy code. The goal here should be to illustrate pyodbc doing the thing you want, using it directly. once that's settled, then any SQLAlchemy issues that interfere with correct pyodbc use can be cleared up on our end, but that can all be skipped by using the pyodbc cursor, as documented at https://docs.sqlalchemy.org/en/20/core/connections.html#working-with-the-dbapi-cursor-directly.

nishantvarma commented 8 months ago

@mkleehammer ,

Testcase:

def test_failure(session):
    product = Product(name="Dettol")
    session.add(Sales(product=product))
    session.flush()
    try:
        session.execute("SELECT 1/0")
    except:
        assert session.query(Sales).one()
    try:
        session.execute("SELECT CAST('A' AS INT)")  # This record exists.
    except:
        assert session.query(Sales).one()  # Fails. I want a way to detect this using xact_state.

ODBC log:

[ODBC][967914][1698374195.982820][__handles.c][460]
                Exit:[SQL_SUCCESS]
                        Environment = 0x55fe3cc6c580
[ODBC][967914][1698374195.982835][SQLSetEnvAttr.c][189]
                Entry:
                        Environment = 0x55fe3cc6c580
                        Attribute = SQL_ATTR_ODBC_VERSION
                        Value = 0x3
                        StrLen = 4
[ODBC][967914][1698374195.982839][SQLSetEnvAttr.c][381]
                Exit:[SQL_SUCCESS]
[ODBC][967914][1698374195.982842][SQLAllocHandle.c][377]
                Entry:
                        Handle Type = 2
                        Input Handle = 0x55fe3cc6c580
[ODBC][967914][1698374195.982846][SQLAllocHandle.c][493]
                Exit:[SQL_SUCCESS]
                        Output Handle = 0x55fe3cc7ee60
[ODBC][967914][1698374195.983197][SQLDriverConnectW.c][290]
                Entry:
                        Connection = 0x55fe3cc7ee60
                        Window Hdl = (nil)
                        Str In = [DRIVER={ODBC Driver 17 for SQL Server};Server=mssql,1433;Database=master;UID=SA;PWD=P@ssword][length = 92 (SQL_NTS)]
                        Str Out = (nil)
                        Str Out Max = 0
                        Str Out Ptr = (nil)
                        Completion = 0
                UNICODE Using encoding ASCII 'UTF-8' and UNICODE 'UCS-2LE'

[ODBC][967914][1698374195.991003][__handles.c][460]
                Exit:[SQL_SUCCESS]
                        Environment = 0x55fe3cc85fb0
[ODBC][967914][1698374195.991044][SQLGetEnvAttr.c][157]
                Entry:
                        Environment = 0x55fe3cc85fb0
                        Attribute = 65002
                        Value = 0x7ffed7332c10
                        Buffer Len = 128
                        StrLen = 0x7ffed7332bac
[ODBC][967914][1698374195.991050][SQLGetEnvAttr.c][273]
                Exit:[SQL_SUCCESS]
[ODBC][967914][1698374195.991056][SQLFreeHandle.c][220]
                Entry:
                        Handle Type = 1
                        Input Handle = 0x55fe3cc85fb0

It would be good to have a fix for this problem. It creates bugs when we use that (bad) try-except pattern.

nishantvarma commented 8 months ago

I came up with a small workaround to achieve something close. The problem is the inability to retrieve SELECT XACT_STATE() from the "current" transaction, because ODBC creates a new one when a failure happens. If we track the transaction id and query the status table, the desired result is achieved:

def get_trans_id(session):
    query = "SELECT transaction_id FROM sys.dm_tran_current_transaction"
    return session.execute(query).scalar()

def get_trans_status(session, tid):
    query = f"""
    SELECT
        XACT_STATE()
    FROM
        sys.dm_tran_database_transactions
    WHERE
        transaction_id = {tid}
    """
    return session.execute(query).scalar()

def test_transaction_failure(session):
    tid = get_trans_id(session)
    product = Product(name="Dettol")
    session.add(Sales(product=product))
    assert get_trans_status(session, tid) == 1
    try:
        # Level < 16 error.
        session.execute("SELECT 1/0")
    except:
        assert get_trans_status(session, tid) == 1
    try:
        # Level >= 16 error.
        session.execute("SELECT CAST('A' AS INT)")
    except:
        assert get_trans_status(session, tid) == None

I expected it to return -1 or 0; but it returns None. I think this is because it is no longer an active transaction (not the same aborted).

mkleehammer commented 8 months ago

@nishantvarma Did you determine this is simply a limitation of ODBC and we should close this?

nishantvarma commented 8 months ago

Agreed, @mkleehammer. Closing this.