From: Mike Bayer Date: Wed, 11 Nov 2020 16:13:27 +0000 (-0500) Subject: Warn / raise for returning() / return_defaults() combinations X-Git-Tag: rel_1_3_21~17 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=85cf381e51df99b8260683c0367b653c419a5b6e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..6180e771da --- /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 92fb4a607a..954ad4381c 100644 --- 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 aa10626c5c..8a3cc64933 100644 --- 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