-
Notifications
You must be signed in to change notification settings - Fork 565
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
Allow customization of default odbc version #1278
Conversation
Example code and error that fails without this PR. It passes with it and 3.8 set.
odbcinst.ini:
|
What I notice about the repro code is that it seems to assume that pyodbc explicitly closes a connection created by a context manager. That is not the case, ref: #48 While I can reproduce the error using the original repro code (with pyodbc 4.0.39), changing it to table = """
CREATE TABLE #TemporaryTable
(
Col1 int,
Col2 varchar(10)
);
"""
connection = pyodbc.connect(connection_string)
connection.execute(table)
connection.close()
connection = pyodbc.connect(connection_string)
connection.execute(table)
connection.close()
connection = pyodbc.connect(connection_string)
connection.execute(table)
connection.close()
connection = pyodbc.connect(connection_string)
connection.execute(table)
connection.close()
connection = pyodbc.connect(connection_string)
connection.execute(table)
connection.close() does not reproduce the error. The difference is that the original repro code results in the
while explicitly closing the connection adds
I also notice that the original repro code fails on the third context manager, not the second, which makes me suspect that "closing" a connection by clobbering the existing TL;DR - I'm just a bit confused. Perhaps @v-chojas might have some insights on this. |
I'll certainly wait on the expert to comment but I'll add my 2c here also. Even if the particular issue demoed can be resolved by using the connection correctly I'd think we would want the ability for the connection to be reset when it gets added back into the pool which only happens with 3.8. Some of the items it does are not covered by the endtran call. Additionally if a connection leaks (not calling close) accidentally you won't end up with a dirty connection. Also, while I demoed this issue using pyodbc directly I stumbled across it using sqlalchemy with the nullpool because we prefer unixodbc pooling. I would be shocked if they were missing the close call, but I'll dig into it. |
Leaving for posterity but this behavior is groked by the comment below. @gordthompson now I'm really confused. With 4.0.39 this produces the error even though the connection is being closed and it's ugly python exploiting weird scoping:
The traces are identical until the very end where the non working case has 2 SqlEndTran entries in the log the first of which has a Completion Type of 0. nonworking:
working:
notice the Completion type of 0 that is only in the non working example (and is 1 in the same spot in the working example):
followed by the Completion Type of 1 that is in both, but is in a different location in the trace compared to the working example:
|
@gordthompson I think you have a bug in your test case. Because autocommit is not enabled the close call in your case rolls back the transaction that creates the temporary table. Therefore it is not associated with the session any longer. If you turn on autocommit or add a commit statement the issue reproduces fine:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Thanks. In the future, we might want to add an error message if the user tries something unsupported like "3.9". |
Fixes #1277
My cpp is VERY rusty. Let me know if there is a better solution for this.