Switch sqlalchemy-cratedb from dev branch to released 0.43.1 - #1867
Conversation
…ps for async dialects
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi Bilal. Thank you. This patch roughly looks like a duplicate of this other one? |
| def test_insert_efficient_multirow(): | ||
| insert_records = 25_000 | ||
| # CrateDB enforces statement_max_length=262144; keep well under that limit. | ||
| insert_records = 5_000 | ||
| cmd = f"time python insert_efficient.py cratedb multirow {insert_records}" | ||
| run(cmd) | ||
|
|
||
|
|
||
| def test_insert_efficient_batched(): | ||
| insert_records = 50_000 | ||
| insert_records = 5_000 |
There was a problem hiding this comment.
Thank you for adding this. I've seen the same thing, and it might be a huge red flag against the recent change in crate-python, so we might not want to have this slip unnoticed? In GH-1839, I am deliberately using crate>=2.1.2,<2.2 and sqlalchemy-cratedb[all]<0.43, in order to surface this discovery on behalf of a later PR and flag it for review more prominently.
I think the change is significant for some users who use the manyrows marshalling style (default with pandas), as it will decrease available bulk sizes in standard communication paths with CrateDB by a huge factor.
In this spirit, I was planning two phases here:
a) Restore compatibility (phase out experimental features) with GH-1839.
b) Surface the bulk size regression with a later patch that upgrades package versions.
c) Present the regression to other colleagues.
wdyt?
There was a problem hiding this comment.
We are tracking this observation here.
There was a problem hiding this comment.
I explained details about this on the related issue page, This is not a bug so there is nothing to do. Just adjusting example codes are enough. Thanks.
Summary of the changes / Why this is an improvement
The SQLAlchemy and pandas nightly CI jobs were failing with:
Both examples were pinned to the
amo/postgresql-asyncdev branch ofsqlalchemy-cratedb, which adds a_format_queryhelper that rewrites?placeholders to%swhen the dialect'sparamstyleispyformat. This branch is old and does not have features that used in current package.This update will fix the pipeline and will help to test with latest packages.
Checklist