Undocumented breaking change (or bug?) in 3.15.0

[sztamas]

Hi @coleifer ,

First of all thanks for your work on peewee :pray: !

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 to returning(). 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 with 3.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 :arrow_down: are passing:

diff --git a/tests/models.py b/tests/models.py
index 5271eeab..15ed41eb 100644
--- a/tests/models.py
+++ b/tests/models.py
@@ -421,8 +421,6 @@ class TestModelAPIs(ModelTestCase):
         User.create(username='u0')  # Ensure that last insert ID != rowcount.

         iq = User.insert_many([(u,) for u in ('u1', 'u2', 'u3')])
-        if IS_POSTGRESQL or IS_CRDB:
-            iq = iq.returning()
         self.assertEqual(iq.as_rowcount().execute(), 3)

         # Now explicitly specify empty returning() for all DBs.
@@ -433,8 +431,6 @@ class TestModelAPIs(ModelTestCase):
                  .select(User.username.concat('-x'))
                  .where(User.username.in_(['u1', 'u2'])))
         iq = User.insert_from(query, ['username'])
-        if IS_POSTGRESQL or IS_CRDB:
-            iq = iq.returning()
         self.assertEqual(iq.as_rowcount().execute(), 2)

         query = (User

Thanks,

Tamas

[coleifer]

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!

[sztamas]

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. Thanks!