mkleehammer / pyodbc

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

Please change default parameter for autocommit to true #1024

Closed davidbaniadam closed 2 years ago

davidbaniadam commented 2 years ago

If you could change the default value for autocommit to true, it would help many database servers and DBAs. E.g. in SQL Server I often see a python connection in a sleeping mode but with an open transaction holding locks on metadata on tempdb. This causes a lot of blocking issues on the instance.

It it really easy for people to forget to commit or leave the default value for autocommit. And as mentioned it creates a lot of blocking issue.

Thank you

v-chojas commented 2 years ago

That would be a big breaking change; the default wasn't pyODBC's decision:

https://www.python.org/dev/peps/pep-0249/#connection-methods "Note that if the database supports an auto-commit feature, this must be initially off. An interface method may be provided to turn it back on."

Note that you could ask the author of that specification why it was like that, and tell him your opinion of that decision.

davidbaniadam commented 2 years ago

@v-chojas: hi I don't know so much about Python packages or libraries. The thing you mention is for the method. I am talking about cnxn = pyodbc.connect('DRIVER={ODBC Driver 17 for SQL Server};SERVER='+server+';DATABASE='+database+';UID='+username+';PWD='+ password' + '; autocommit=False)'

The connect function has a parameter called autocommit. I am suggesting to set that to True. Don't see why it is a breaking change.

v-chojas commented 2 years ago

It is a breaking change because all other applications using pyODBC will be expecting it false.

davidbaniadam commented 2 years ago

@v-chojas: The way I see it the only problem that could arise is that multi statement transactions, with autocommit =true , will commit each statement but some transactions might have more than one statement. So yes, it could make some code out there not function as intended.

When autocommit is true, it does not seem to cause a problem even if there is an explicit commit.

I don't think the majority of users of this package write multi statement transactions.
It seems most users are doing data analysis with SELECT, perhaps creating temporary tables. In any case I would argue one should not rely on a default setting when writing multi statement transactions.

I think it would be a huge deal if the default for autocommit was is changed to true. It is really easy for people to forget to include an explicit commit. There most be a way to introduce breaking changes, perhaps with warnings for a period?

davidbaniadam commented 2 years ago

@v-chojas: The SQL Standard defines an implicit transaction mode which is supposed be the default mode. Actually like the default setting in this package. Perhaps this is the reason for the choice of default values here.

mkleehammer commented 2 years ago

It is absolutely a breaking change and is against the DB API specification. I can't change this, and over time most people find that it is a very good default.

You can also change it on the fly using cnxn.autocommit = True.