Migrate to paramstyle="pyformat", following crate-python 2.2.0#266
Migrate to paramstyle="pyformat", following crate-python 2.2.0#266amotl wants to merge 14 commits into
paramstyle="pyformat", following crate-python 2.2.0#266Conversation
This comment was marked as spam.
This comment was marked as spam.
amotl
force-pushed
the
paramstyle-pyformat
branch
from
fec5468 to
c58b587
Compare
|
Hi @bgunebakan. Please let me know if you think it makes sense to take over this PR, or otherwise close it without much ado at your disposal if relevant fixes are coming from upstream crate-python, so this patch might no longer be needed. Thanks! |
| dependencies = [ | ||
| "backports.zoneinfo<1; python_version<'3.9'", | ||
| "crate>=2,<3", | ||
| "crate>=2.2.1b2,<3", |
There was a problem hiding this comment.
IMPORTANT: upgrade to stable version 2.2.1
| "crate>=2.2.1b2,<3", | |
| "crate>=2.2.1,<3", |
kneth
left a comment
kneth
left a comment
There was a problem hiding this comment.
It is a good change, but we need to explain it carefully in CHANGES.md as it has a big impact for the users' code.
Maybe bring in a major version bump at the same time to signal this? |
Thanks, I extended |
mfussenegger
left a comment
mfussenegger
left a comment
There was a problem hiding this comment.
Left a suggestion for the changes entry.
Otherwise lgtm - but would be great if this could be squashed into 2 commits: One that drops support for the outdated python versions and another one that bumps crate-python + makes the pyformat changes.
| - Migrated to `paramstyle="pyformat"`, following crate-python 2.2.1. | ||
|
|
||
| **Breaking change:** The dialect now generates `%(name)s` named placeholders | ||
| and passes a `dict` of parameters to the DBAPI cursor, instead of the previous | ||
| `?` positional placeholders with a `tuple`. crate-python converts these to | ||
| CrateDB's native `$N` positional markers before sending over HTTP, so no | ||
| server-side changes are required. | ||
|
|
||
| User impact: | ||
|
|
||
| - **ORM / Core users**: No action required. SQLAlchemy handles parameter | ||
| binding transparently; existing application code continues to work unchanged. | ||
| - **Direct cursor users** who log or inspect the SQL string passed to | ||
| `cursor.execute()` will observe `%(name)s` placeholders instead of `?`. | ||
| - **Partial `ObjectType` updates** generate subscript-assignment SQL with named | ||
| parameters, e.g. `SET data['x'] = %(data_'x'_)s`, replacing the previous | ||
| positional form `SET data['x'] = ?`. | ||
| - **`pandas` / Dask `DataFrame.to_sql(..., method=insert_bulk)`** continues to | ||
| work. The bulk-operations path sends `$N` positional SQL to CrateDB, with | ||
| conversion handled by crate-python ≥ 2.2.1. | ||
| - **Minimum Python version raised to 3.10.** crate-python 2.1.0+ requires | ||
| Python ≥ 3.10. Python 3.6–3.9 are no longer supported. |
There was a problem hiding this comment.
| - Migrated to `paramstyle="pyformat"`, following crate-python 2.2.1. | |
| **Breaking change:** The dialect now generates `%(name)s` named placeholders | |
| and passes a `dict` of parameters to the DBAPI cursor, instead of the previous | |
| `?` positional placeholders with a `tuple`. crate-python converts these to | |
| CrateDB's native `$N` positional markers before sending over HTTP, so no | |
| server-side changes are required. | |
| User impact: | |
| - **ORM / Core users**: No action required. SQLAlchemy handles parameter | |
| binding transparently; existing application code continues to work unchanged. | |
| - **Direct cursor users** who log or inspect the SQL string passed to | |
| `cursor.execute()` will observe `%(name)s` placeholders instead of `?`. | |
| - **Partial `ObjectType` updates** generate subscript-assignment SQL with named | |
| parameters, e.g. `SET data['x'] = %(data_'x'_)s`, replacing the previous | |
| positional form `SET data['x'] = ?`. | |
| - **`pandas` / Dask `DataFrame.to_sql(..., method=insert_bulk)`** continues to | |
| work. The bulk-operations path sends `$N` positional SQL to CrateDB, with | |
| conversion handled by crate-python ≥ 2.2.1. | |
| - **Minimum Python version raised to 3.10.** crate-python 2.1.0+ requires | |
| Python ≥ 3.10. Python 3.6–3.9 are no longer supported. | |
| - The dialect now uses `paramstyle = "pyformat"` supported in crate-python 2.2.1. | |
| Because of this the SQL statements logged or inspected will contain | |
| `%(name)s` placeholders instead of `?`. | |
| - Removed support for Python versions < 3.11 |
Mentioning 3 internal changes that basically boil down to "nothing changes for the user" seems a bit too verbose.
Out of curiosity: Was this LLM generated?
There was a problem hiding this comment.
Hi. I had the impression the change would cause some breaking changes, so I requested them being mentioned within the changelog. Apologies if this is not the case at all.
There was a problem hiding this comment.
Mentioning 3 internal changes that basically boil down to "nothing changes for the user" seems a bit too verbose.
Out of curiosity: Was this LLM generated?
I gathered information with LLM and build the list based on changes.
4 participants
About
A few adjustments were necessary to accommodate the test suite to upstream changes in crate-python 2.2.0. The updates show that the improvement might have an impact to downstream users, so we might want to handle it with more care even in retrospective.
References