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

Transaction rollback seems to be unsuccessful #425

Open
jakehu opened this issue Nov 19, 2021 · 13 comments
Open

Transaction rollback seems to be unsuccessful #425

jakehu opened this issue Nov 19, 2021 · 13 comments

Comments

@jakehu
Copy link

jakehu commented Nov 19, 2021

Below is my code

async with conn.transaction(force_rollback=True):
    transaction = await conn.transaction()
    try:
        query = user.insert().values(**row["data"])
        await conn.execute(query=query)

        query_sms = (
            user_sms.update()
            .where(user_sms.c.ccode == "code")
            .values(status=1)
        )
        await conn.execute(query=query_sms)
    except Exception as e:
        print(e)
        print("This is the error")
        await transaction.rollback()
        return False
    else:
        await transaction.commit()
        return True

I estimate that user_sms.c.code is written as user_sms.c.ccode

As a result, user.insert has been executed, but user_sms.update has not been executed

The print function outputs, but the single database does not roll back

print output:

ccode
This is the error

depend:
databases==0.5.3
SQLAlchemy==1.4.25

@aminalaee
Copy link
Member

I think this is the same as #403.

@jakehu
Copy link
Author

jakehu commented Nov 22, 2021

So the two executed transactions are not the same connection?
So, how should we solve it?

@aminalaee
Copy link
Member

I'm not really sure how to fix it.

The only way I could fix it was by reverting parallel transactions here.

Still waiting for a reply there.

@jakehu
Copy link
Author

jakehu commented Nov 24, 2021

OH NO this is really bad news

@ffunenga
Copy link

@jakehu Why to do you need the first line?

async with conn.transaction(force_rollback=True):
    # (...)

Would your code do what you want if you removed the with statement and decreased the indentation of the rest of the code?

Another idea to consider:

try:
    async with conn.transaction():
        query = user.insert().values(**row["data"])
        await conn.execute(query=query)

        query_sms = (
            user_sms.update()
            .where(user_sms.c.ccode == "code")
            .values(status=1)
        )
        await conn.execute(query=query_sms)
except Exception as e:
    print(e)
    print("This is the error")
    return False
else:
    return True

(I haven't tested this idea. I am sorry in advance if it doesn't work. In theory, it seems OK [famous last words])

@jakehu
Copy link
Author

jakehu commented Dec 14, 2021

Plus with is to refer to https://github.com/encode/databases/blob/master/tests/test_databases.py line 589

It is also unsuccessful to change it to what you wrote. I found a problem. The problem should come from conn.

When I changed to:

async with Database(db_url) as conn:
    async with conn.transaction(force_rollback=True):
        transaction = await conn.transaction()
        try:
            query = user.insert().values(**row["data"])
            await conn.execute(query=query)

            query_sms = (
                user_sms.update()
                .where(user_sms.c.ccode == "code")
                .values(status=1)
            )
            await conn.execute(query=query_sms)
        except Exception as e:
            print(e)
            print("This is the error")
            await transaction.rollback()
            return False
        else:
            await transaction.commit()
            return True

Can run

And my previous conn is defined like this

database = Database(app.config.custom["mysql"])

@app.listener("before_server_start")
async def startup_db(app, loop):
     await database.connect()

@app.listener("after_server_stop")
async def shutdown_db(app, loop):
     await database.disconnect()

app.ctx.conn = database

Write the connection link to the application context

@ffunenga @aminalaee

@aminalaee
Copy link
Member

@jakehu
I guess that test uses nested transactions, specifically database.transaction(force_rollback=True (the outer one) to rollback when test finishes.

@jakehu
Copy link
Author

jakehu commented Dec 14, 2021

I tested it, my writing above is still wrong, it still can’t run
I don't know what you said

@jakehu
Copy link
Author

jakehu commented Dec 15, 2021

@aminalaee
Finally I referenced #424
Modified with transaction._connection

transaction = await conn.transaction()
try:
    query = user.insert().values(**row["data"])
    await transaction._connection.execute(query=query)

    query_sms = (
        user_sms.update()
        .where(user_sms.c.ccode == "code")
        .values(status=1)
    )
    await transaction._connection.execute(query=query_sms)
except Exception as e:
    print(e)
    print("This is the error")
    await transaction.rollback()
    return False
else:
    await transaction.commit()
    return True

It works, but I don't know if it is correct?

@aminalaee
Copy link
Member

I think that's not related, this should work too:

transaction = await conn.transaction()
try:
    query = user.insert().values(**row["data"])
    await conn.execute(query=query)

    query_sms = (
        user_sms.update()
        .where(user_sms.c.ccode == "code")
        .values(status=1)
    )
    await conn.execute(query=query_sms)
except Exception as e:
    print(e)
    print("This is the error")
    await transaction.rollback()
    return False
else:
    await transaction.commit()
    return True

@jakehu
Copy link
Author

jakehu commented Dec 17, 2021

I know, but it really doesn't work

@aminalaee
Copy link
Member

@jakehu
Cqn you test and post traceback of when you run this sample.

@alex-pobeditel-2004
Copy link

Looks like it was fixed in 0.6.0 🎉

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

No branches or pull requests

4 participants