Warn / raise for returning() / return_defaults() combinations

A warning is emmitted if a returning() method such as
:meth:`_sql.Insert.returning` is called multiple times, as this does not
yet support additive operation.  Version 1.4 will support additive
operation for this.  Additionally, any combination of the
:meth:`_sql.Insert.returning` and :meth:`_sql.Insert.return_defaults`
methods now raises an error as these methods are mutually exclusive;
previously the operation would fail silently.

Fixes: #5691
Change-Id: Id95e0f9da48bba0b59439cb26564f0daa684c8e3
(cherry picked from commit 44d64aef3ce26a1dd62b61ed15888d8baab06443)

diff --git a/doc/build/changelog/unreleased_13/5691.rst b/doc/build/changelog/unreleased_13/5691.rst
new file mode 100644 (file)
index 0000000..6180e77
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5691.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 5691
+
+    A warning is emmitted if a returning() method such as
+    :meth:`_sql.Insert.returning` is called multiple times, as this does not
+    yet support additive operation.  Version 1.4 will support additive
+    operation for this.  Additionally, any combination of the
+    :meth:`_sql.Insert.returning` and :meth:`_sql.Insert.return_defaults`
+    methods now raises an error as these methods are mutually exclusive;
+    previously the operation would fail silently.
+

diff --git a/lib/sqlalchemy/sql/dml.py b/lib/sqlalchemy/sql/dml.py
index 92fb4a607ac3fac90b836d08068b83a81f2c866c..954ad4381c1c6d1e1be022114c918bf9cb3f4357 100644 (file)
--- a/lib/sqlalchemy/sql/dml.py
+++ b/lib/sqlalchemy/sql/dml.py
@@ -42,6 +42,7 @@ class UpdateBase(
     _parameter_ordering = None
     _prefixes = ()
     named_with_column = False
+    _return_defaults = None
 
     def _process_colparams(self, parameters):
         def process_single(p):
@@ -119,11 +120,10 @@ class UpdateBase(
             for server_flag, updated_timestamp in connection.execute(stmt):
                 print(server_flag, updated_timestamp)
 
-        The given collection of column expressions should be derived from
-        the table that is
-        the target of the INSERT, UPDATE, or DELETE.  While
-        :class:`_schema.Column`
-        objects are typical, the elements can also be expressions::
+        The given collection of column expressions should be derived from the
+        table that is the target of the INSERT, UPDATE, or DELETE.  While
+        :class:`_schema.Column` objects are typical, the elements can also be
+        expressions::
 
             stmt = table.insert().returning(
                 (table.c.first_name + " " + table.c.last_name).
@@ -159,6 +159,16 @@ class UpdateBase(
 
 
         """
+        if self._return_defaults:
+            raise exc.InvalidRequestError(
+                "return_defaults() is already configured on this statement"
+            )
+        if self._returning:
+            util.warn(
+                "The returning() method does not currently support multiple "
+                "additive calls.  The existing RETURNING clause being "
+                "replaced by new columns."
+            )
         self._returning = cols
 
     @_generative
@@ -476,6 +486,10 @@ class ValuesBase(UpdateBase):
             :attr:`_engine.ResultProxy.returned_defaults`
 
         """
+        if self._returning:
+            raise exc.InvalidRequestError(
+                "RETURNING is already configured on this statement"
+            )
         self._return_defaults = cols or True
 
 

diff --git a/test/sql/test_returning.py b/test/sql/test_returning.py
index aa10626c5c853aea3d634f3d5d39ef9666f31fe5..8a3cc64933561adb7a4e455fcd916f2ba87452d7 100644 (file)
--- a/test/sql/test_returning.py
+++ b/test/sql/test_returning.py
@@ -1,15 +1,19 @@
 import itertools
 
 from sqlalchemy import Boolean
+from sqlalchemy import delete
 from sqlalchemy import exc as sa_exc
 from sqlalchemy import func
+from sqlalchemy import insert
 from sqlalchemy import Integer
 from sqlalchemy import MetaData
 from sqlalchemy import select
 from sqlalchemy import Sequence
 from sqlalchemy import String
 from sqlalchemy import testing
+from sqlalchemy import update
 from sqlalchemy.testing import assert_raises_message
+from sqlalchemy.testing import AssertsCompiledSQL
 from sqlalchemy.testing import AssertsExecutionResults
 from sqlalchemy.testing import engines
 from sqlalchemy.testing import eq_
@@ -22,6 +26,76 @@ from sqlalchemy.types import TypeDecorator
 table = GoofyType = seq = None
 
 
+class ReturnCombinationTests(fixtures.TestBase, AssertsCompiledSQL):
+    __dialect__ = "postgresql"
+
+    @testing.fixture
+    def table_fixture(self):
+        return Table(
+            "foo",
+            MetaData(),
+            Column("id", Integer, primary_key=True),
+            Column("q", Integer, server_default="5"),
+            Column("x", Integer),
+            Column("y", Integer),
+        )
+
+    @testing.combinations(
+        (
+            insert,
+            "INSERT INTO foo (id, q, x, y) "
+            "VALUES (%(id)s, %(q)s, %(x)s, %(y)s)",
+        ),
+        (update, "UPDATE foo SET id=%(id)s, q=%(q)s, x=%(x)s, y=%(y)s"),
+        (delete, "DELETE FROM foo"),
+        argnames="dml_fn, sql_frag",
+        id_="na",
+    )
+    def test_return_combinations(self, table_fixture, dml_fn, sql_frag):
+        t = table_fixture
+        stmt = dml_fn(t)
+
+        stmt = stmt.returning(t.c.x)
+
+        with testing.expect_warnings(
+            r"The returning\(\) method does not currently "
+            "support multiple additive calls."
+        ):
+            stmt = stmt.returning(t.c.y)
+
+        self.assert_compile(
+            stmt,
+            "%s RETURNING foo.y" % (sql_frag),
+        )
+
+    def test_return_no_return_defaults(self, table_fixture):
+        t = table_fixture
+
+        stmt = t.insert()
+
+        stmt = stmt.returning(t.c.x)
+
+        assert_raises_message(
+            sa_exc.InvalidRequestError,
+            "RETURNING is already configured on this statement",
+            stmt.return_defaults,
+        )
+
+    def test_return_defaults_no_returning(self, table_fixture):
+        t = table_fixture
+
+        stmt = t.insert()
+
+        stmt = stmt.return_defaults()
+
+        assert_raises_message(
+            sa_exc.InvalidRequestError,
+            r"return_defaults\(\) is already configured on this statement",
+            stmt.returning,
+            t.c.x,
+        )
+
+
 class ReturningTest(fixtures.TestBase, AssertsExecutionResults):
     __requires__ = ("returning",)
     __backend__ = True