-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Undocumented breaking change (or bug?) in 3.15.0 #2876
Comments
sztamas
changed the title
Breaking change or bug in 3.15.0
Undocumented breaking change (or bug?) in 3.15.0
Apr 22, 2024
Yeah we'll keep the new behavior, but I've updated the changelog and removed the unneeded code from the tests which you highlighted. Thank you! |
That was quick! :-) The only other thing I would consider changing is: diff --git a/docs/peewee/api.rst b/docs/peewee/api.rst
index 421c45f2..4d46cd4c 100644
--- a/docs/peewee/api.rst
+++ b/docs/peewee/api.rst
@@ -4231,7 +4231,7 @@ Model
The default return value is the number of rows modified. However,
when using Postgres, Peewee will return a cursor by default that
yields the primary-keys of the inserted rows. To disable this
- functionality with Postgres, use an empty call to ``returning()``.
+ functionality with Postgres, use ``as_rowcount()``.
.. py:classmethod:: insert_from(query, fields) I would expect that "Disabling the Postgres specific functionality" in this context would mean that you would get the default return value ie. the row-count and not the cursor. |
coleifer
added a commit
that referenced
this issue
Apr 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi @coleifer ,
First of all thanks for your work on
peewee
🙏 !The
3.15.0
release announced a breaking change that affected people relying on bulk-inserts returning the row-count on MySQL and Sqlite.What has been unfortunately missed, is that the changes introduced also broke the Postgresql API.
Up to version
3.14.10
, the way to receive the row-count on Postgresql bulk-inserts was to use an empty call toreturning()
.Starting with the changes in
3.15.0
, the same exact call will return the Postgresql cursor instead of the row-count.I started to work on a PR, that updates the documentation and the Release notes of
3.15.0
to reflect this, but then I realized that I'm not sure if you want to leave everything as is and update the docs, or also handle this as a bug and "fix"returning()
to be backwards-compatible for Postgresql.I initially assumed that the API would recommend using the new
as_rowcount()
method to be used on all DBs and document that this is the new way of receiving the row-count starting with3.15.0
.BTW, is there a point of calling or recommending calling
returning().as_rowcount()
for Postgresql instead of just simply.as_rowcount()
uniformly for all DBs?These tests ⬇️ are passing:
Thanks,
Tamas
The text was updated successfully, but these errors were encountered: