From 2f62fb12ea44d552bf95903c7d82f6952cdbf026 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 8 Jun 2023 17:40:01 -0400 Subject: [PATCH] ensure primary key values all present for ORM bulk UPDATE Fixed bug in new feature where ORM bulk UPDATE can be combined with a WHERE clause, added in version 2.0.11 as part of :ticket:`9583`, where sending dictionaries that did not include the primary key values for each row would run through the bulk process and include "pk=NULL" for the rows, silently failing. An exception is now raised if primary key values for bulk UPDATE are not supplied. Additionally, document WHERE criteria feature as well as how to invoke an UPDATE outside of "bulk orm by primary key" Fixes: #9917 Change-Id: Ie8e0d604050f0dfbab77473020771383c087d14a --- doc/build/changelog/unreleased_20/9917.rst | 10 +++ doc/build/errors.rst | 49 ++++++++++++++ doc/build/orm/queryguide/dml.rst | 74 ++++++++++++++++++---- lib/sqlalchemy/orm/persistence.py | 12 ++++ test/orm/dml/test_bulk_statements.py | 73 ++++++++++++++++++++- 5 files changed, 204 insertions(+), 14 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/9917.rst diff --git a/doc/build/changelog/unreleased_20/9917.rst b/doc/build/changelog/unreleased_20/9917.rst new file mode 100644 index 0000000000..436654d384 --- /dev/null +++ b/doc/build/changelog/unreleased_20/9917.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + :tickets: 9917 + + Fixed bug in new feature which allows a WHERE clause to be used in + conjunction with :ref:`orm_queryguide_bulk_update`, added in version 2.0.11 + as part of :ticket:`9583`, where sending dictionaries that did not include + the primary key values for each row would run through the bulk process and + include "pk=NULL" for the rows, silently failing. An exception is now + raised if primary key values for bulk UPDATE are not supplied. diff --git a/doc/build/errors.rst b/doc/build/errors.rst index 2f2bfa91f4..e02dfcb3fa 100644 --- a/doc/build/errors.rst +++ b/doc/build/errors.rst @@ -1471,6 +1471,55 @@ attention to the rules applied to `inheritance `_. .. _dc_superclass: https://docs.python.org/3/library/dataclasses.html#inheritance +.. _error_bupq: + +per-row ORM Bulk Update by Primary Key requires that records contain primary key values +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This error occurs when making use of the :ref:`orm_queryguide_bulk_update` +feature without supplying primary key values in the given records, such as:: + + + >>> session.execute( + ... update(User).where(User.name == bindparam("u_name")), + ... [ + ... {"u_name": "spongebob", "fullname": "Spongebob Squarepants"}, + ... {"u_name": "patrick", "fullname": "Patrick Star"}, + ... ], + ... ) + +Above, the presence of a list of parameter dictionaries combined with usage of +the :class:`_orm.Session` to execute an ORM-enabled UPDATE statement will +automatically make use of ORM Bulk Update by Primary Key, which expects +parameter dictionaries to include primary key values, e.g.:: + + >>> session.execute( + ... update(User), + ... [ + ... {"id": 1, "fullname": "Spongebob Squarepants"}, + ... {"id": 3, "fullname": "Patrick Star"}, + ... {"id": 5, "fullname": "Eugene H. Krabs"}, + ... ], + ... ) + +To invoke the UPDATE statement without supplying per-record primary key values, +use :meth:`_orm.Session.connection` to acquire the current :class:`_engine.Connection`, +then invoke with that:: + + >>> session.connection().execute( + ... update(User).where(User.name == bindparam("u_name")), + ... [ + ... {"u_name": "spongebob", "fullname": "Spongebob Squarepants"}, + ... {"u_name": "patrick", "fullname": "Patrick Star"}, + ... ], + ... ) + + +.. seealso:: + + :ref:`orm_queryguide_bulk_update` + + :ref:`orm_queryguide_bulk_update_disabling` diff --git a/doc/build/orm/queryguide/dml.rst b/doc/build/orm/queryguide/dml.rst index 04a4fb2bb0..5c7acb2086 100644 --- a/doc/build/orm/queryguide/dml.rst +++ b/doc/build/orm/queryguide/dml.rst @@ -654,13 +654,16 @@ ORM, using an explicit WHERE clause, which is documented at For the "bulk" version of UPDATE, a :func:`_dml.update` construct is made in terms of an ORM class and passed to the :meth:`_orm.Session.execute` method; -the resulting :class:`_dml.Update` object should have **no WHERE criteria or -values**, that is, the :meth:`_dml.Update.where` and :meth:`_dml.Update.values` -methods are not used. Passing the :class:`_dml.Update` construct along with a -list of parameter dictionaries which each include a full primary key value will -invoke **bulk UPDATE by primary key mode** for the statement, generating the -appropriate WHERE criteria to match each row by primary key, and using -:term:`executemany` to run each parameter set against the UPDATE statement:: +the resulting :class:`_dml.Update` object should have **no values and typically +no WHERE criteria**, that is, the :meth:`_dml.Update.values` method is not +used, and the :meth:`_dml.Update.where` is **usually** not used, but may be +used in the unusual case that additional filtering criteria would be added. + +Passing the :class:`_dml.Update` construct along with a list of parameter +dictionaries which each include a full primary key value will invoke **bulk +UPDATE by primary key mode** for the statement, generating the appropriate +WHERE criteria to match each row by primary key, and using :term:`executemany` +to run each parameter set against the UPDATE statement:: >>> from sqlalchemy import update >>> session.execute( @@ -675,10 +678,19 @@ appropriate WHERE criteria to match each row by primary key, and using [...] [('Spongebob Squarepants', 1), ('Patrick Star', 3), ('Eugene H. Krabs', 5)] {stop}<...> +Note that each parameter dictionary **must include a full primary key for +each record**, else an error is raised. + Like the bulk INSERT feature, heterogeneous parameter lists are supported here as well, where the parameters will be grouped into sub-batches of UPDATE runs. +.. versionchanged:: 2.0.11 Additional WHERE criteria can be combined with + :ref:`orm_queryguide_bulk_update` by using the :meth:`_dml.Update.where` + method to add additional criteria. However this criteria is always in + addition to the WHERE criteria that's already made present which includes + primary key values. + The RETURNING feature is not available when using the "bulk UPDATE by primary key" feature; the list of multiple parameter dictionaries necessarily makes use of DBAPI :term:`executemany`, which in its usual form does not typically @@ -686,12 +698,48 @@ support result rows. .. versionchanged:: 2.0 Passing an :class:`_dml.Update` construct to the - :meth:`_orm.Session.execute` method along with a list of parameter dictionaries - and no WHERE criteria now invokes a "bulk update", which - makes use of the same functionality as the legacy - :meth:`_orm.Session.bulk_update_mappings` method. This is a behavior change - compared to the 1.x series where the :class:`_dml.Update` would only be - supported with explicit WHERE criteria and inline VALUES. + :meth:`_orm.Session.execute` method along with a list of parameter + dictionaries now invokes a "bulk update", which makes use of the same + functionality as the legacy :meth:`_orm.Session.bulk_update_mappings` + method. This is a behavior change compared to the 1.x series where the + :class:`_dml.Update` would only be supported with explicit WHERE criteria + and inline VALUES. + +.. _orm_queryguide_bulk_update_disabling: + +Disabling Bulk ORM Update by Primary Key for an UPDATE statement with multiple parameter sets +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ORM Bulk Update by Primary Key feature, which runs an UPDATE statement +per record which includes WHERE criteria for each primary key value, is +automatically used when: + +1. the UPDATE statement given is against an ORM entity +2. the :class:`_orm.Session` is used to execute the statement, and not a + Core :class:`_engine.Connection` +3. The parameters passed are a **list of dictionaries**. + +In order to invoke an UPDATE statement without using "ORM Bulk Update by Primary Key", +invoke the statement against the :class:`_engine.Connection` directly using +the :meth:`_orm.Session.connection` method to acquire the current +:class:`_engine.Connection` for the transaction:: + + + >>> from sqlalchemy import bindparam + >>> session.connection().execute( + ... update(User).where(User.name == bindparam("u_name")), + ... [ + ... {"u_name": "spongebob", "fullname": "Spongebob Squarepants"}, + ... {"u_name": "patrick", "fullname": "Patrick Star"}, + ... ], + ... ) + {execsql}UPDATE user_account SET fullname=? WHERE user_account.name = ? + [...] [('Spongebob Squarepants', 'spongebob'), ('Patrick Star', 'patrick')] + {stop}<...> + +.. seealso:: + + :ref:`error_bupq` .. _orm_queryguide_bulk_update_joined_inh: diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index 87d7ee7312..31fbdf3438 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -554,6 +554,18 @@ def _collect_update_commands( mapper._pk_attr_keys_by_table[table] ) } + if util.NONE_SET.intersection(pk_params.values()): + raise sa_exc.InvalidRequestError( + f"No primary key value supplied for column(s) " + f"""{ + ', '.join( + str(c) for c in pks if pk_params[c._label] is None) + }; """ + "per-row ORM Bulk UPDATE by Primary Key requires that " + "records contain primary key values", + code="bupq", + ) + else: pk_params = {} for col in pks: diff --git a/test/orm/dml/test_bulk_statements.py b/test/orm/dml/test_bulk_statements.py index f10a38ff45..9550671119 100644 --- a/test/orm/dml/test_bulk_statements.py +++ b/test/orm/dml/test_bulk_statements.py @@ -375,7 +375,7 @@ class InsertStmtTest(testing.AssertsExecutionResults, fixtures.TestBase): eq_(e2.user_name, "e2 new name") -class UpdateStmtTest(fixtures.TestBase): +class UpdateStmtTest(testing.AssertsExecutionResults, fixtures.TestBase): __backend__ = True @testing.variation( @@ -516,6 +516,77 @@ class UpdateStmtTest(fixtures.TestBase): ], ) + @testing.variation("add_where", [True, False]) + @testing.variation("multi_row", ["multirow", "singlerow", "listwsingle"]) + def test_bulk_update_no_pk(self, decl_base, add_where, multi_row): + """test #9917""" + + class A(decl_base): + __tablename__ = "a" + + id: Mapped[int] = mapped_column( + primary_key=True, autoincrement=False + ) + + x: Mapped[int] + y: Mapped[int] + + decl_base.metadata.create_all(testing.db) + + s = fixture_session() + + s.add_all( + [A(id=1, x=1, y=1), A(id=2, x=2, y=2), A(id=3, x=3, y=3)], + ) + s.commit() + + stmt = update(A) + if add_where: + stmt = stmt.where(A.x > 1) + + if multi_row.multirow: + data = [ + {"x": 3, "y": 8}, + {"x": 5, "y": 9}, + {"x": 12, "y": 15}, + ] + + stmt = stmt.execution_options(synchronize_session=None) + elif multi_row.listwsingle: + data = [ + {"x": 5, "y": 9}, + ] + + stmt = stmt.execution_options(synchronize_session=None) + elif multi_row.singlerow: + data = {"x": 5, "y": 9} + else: + multi_row.fail() + + if multi_row.multirow or multi_row.listwsingle: + with expect_raises_message( + exc.InvalidRequestError, + r"No primary key value supplied for column\(s\) a.id; per-row " + "ORM Bulk UPDATE by Primary Key requires that records contain " + "primary key values", + ): + s.execute(stmt, data) + else: + with self.sql_execution_asserter() as asserter: + s.execute(stmt, data) + + if add_where: + asserter.assert_( + CompiledSQL( + "UPDATE a SET x=:x, y=:y WHERE a.x > :x_1", + [{"x": 5, "y": 9, "x_1": 1}], + ), + ) + else: + asserter.assert_( + CompiledSQL("UPDATE a SET x=:x, y=:y", [{"x": 5, "y": 9}]), + ) + def test_bulk_update_w_where_one(self, decl_base): """test use case in #9595""" -- 2.47.2