Always Rollback When Closing/Returning a Connection and Support Connection Subclassing

[dongyx]

1. While closing a connection, even it's just returned to the pool, rollback() should be called to keep consistent with the behavior of real closing and to prevent from chaos between multiple sessions;

2. Support subclassing pymysql.Connection.

[jkklee]

tks @dongyx

1. The intention of this library is to be completely consistent with pymysql, except for the additional logic to maintain the connection pool, and Pymysql does not explicitly perform rollback when closing connections. So I don't think there's much need to add this feature. The manipulation of transactions is up to the developer to care about; after all, explicit is better than implicit.

[dongyx]

1. The behavior of pymysql

The intention of this library is to be completely consistent with pymysql, except for the additional logic to maintain the connection pool, and Pymysql does not explicitly perform rollback when closing connections.

The documentation of pymysql says the behavior of `close()` meets PEP 249 <https://pymysql.readthedocs.io/en/latest/modules/connections.html#pymysql.connections.Connection.close>:

close() Send the quit message and close the socket. See Connection.close() in the specification. Raises:Error – If the connection is already closed.

"the specification" links to PEP249 <https://peps.python.org/pep-0249/#Connection.close> and it says:

Note that closing a connection without committing the changes first will cause an implicit rollback to be performed.

2. Design

after all, explicit is better than implicit.

I agree. Should we add an explicit argument like `auto_rollback` to solve this?

…

On Feb 5, 2024, at 17:37, kee.lee ***@***.***> wrote: • The intention of this library is to be completely consistent with pymysql, except for the additional logic to maintain the connection pool, and Pymysql does not explicitly perform rollback when closing connections. So I don't think there's much need to add this feature. The manipulation of transactions is up to the developer to care about; after all, explicit is better than implicit. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: ***@***.***>

[jkklee]

Note that closing a connection without committing the changes first will cause an implicit rollback to be performed.

As far as I know, this is the action of MySQL，and does not need to handle this at the library software level。

On the other hand, PyMySQL's code does not have an active rollback operation.

[dongyx]

As far as I know, this is the action of MySQL，and does not need to handle this at the library software level。 On the other hand, PyMySQL's code does not have an active rollback operation.

Yes, MySQL automatically rollbacks an uncommitted transaction before closing the connection. Thus PyMySQL needn't explicitly rollback a connection because the close operation already did that. But pymysql-pool's close operation doesn't close the underneath connection to MySQL so the rollback needs to be called explicitly. I understand that you think users should commit or rollback the connection explicitly. One should never return a connection with neither being committed nor rolled back. I agree. But please consider the following snippet which has explicit committing. with conn: with conn.cursor() as cur: sql1 = 'INSERT INTO t1(id) VALUES(1)' sql2 = 'INSERT INTO t1(id) VALUES(2)' cur.execute(sql1) cur.execute(sql2) # Suppose it's a duplicated key. cur.commit() With PyMySQL's connection, the record with id-1 will never be inserted. With pymysql-pool's connection, because a duplicated key raises `pymysql.err.IntegrityError` which is in `_reusable_expection`, this connection will be returned to the pool without rollback. Thus the record with id-1 may be inserted by the next session who happens to take this connection. Because this usage pattern is demonstrated by the documentation of PyMySQL, we have reasons to believe that a number of projects have been written with this pattern, including some of mine. The lack of automatic rollback may prevent those projects from migrating to this wonderful library. I understand that some projects have relied on this library. To maintain backward compatibility, maybe we could provide a parameter for this behavior.