From 8e4e325319eaadb81cc1b6e8c8db7cc1a6b920bd Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 8 Dec 2022 19:31:37 -0500 Subject: [PATCH] add eager_defaults="auto" for inserts Added a new default value for the :paramref:`.Mapper.eager_defaults` parameter "auto", which will automatically fetch table default values during a unit of work flush, if the dialect supports RETURNING for the INSERT being run, as well as :ref:`insertmanyvalues ` available. Eager fetches for server-side UPDATE defaults, which are very uncommon, continue to only take place if :paramref:`.Mapper.eager_defaults` is set to ``True``, as there is no batch-RETURNING form for UPDATE statements. Fixes: #8889 Change-Id: I84b91092a37c4cd216e060513acde3eb0298abe9 --- doc/build/changelog/unreleased_20/8889.rst | 13 + doc/build/changelog/whatsnew_20.rst | 5 + doc/build/orm/persistence_techniques.rst | 242 +++++++++++------- lib/sqlalchemy/dialects/mssql/base.py | 1 + lib/sqlalchemy/orm/mapper.py | 52 +++- lib/sqlalchemy/orm/persistence.py | 37 ++- lib/sqlalchemy/testing/__init__.py | 1 + lib/sqlalchemy/testing/config.py | 53 ++-- test/orm/inheritance/test_basic.py | 64 +++-- test/orm/test_defaults.py | 9 +- test/orm/test_expire.py | 5 +- test/orm/test_unitofwork.py | 21 +- test/orm/test_unitofworkv2.py | 269 +++++++++++++++++++-- 13 files changed, 606 insertions(+), 166 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/8889.rst diff --git a/doc/build/changelog/unreleased_20/8889.rst b/doc/build/changelog/unreleased_20/8889.rst new file mode 100644 index 0000000000..1aedc9fe00 --- /dev/null +++ b/doc/build/changelog/unreleased_20/8889.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: orm, feature + :tickets: 8889 + + Added a new default value for the :paramref:`.Mapper.eager_defaults` + parameter "auto", which will automatically fetch table default values + during a unit of work flush, if the dialect supports RETURNING for the + INSERT being run, as well as + :ref:`insertmanyvalues ` available. Eager fetches + for server-side UPDATE defaults, which are very uncommon, continue to only + take place if :paramref:`.Mapper.eager_defaults` is set to ``True``, as + there is no batch-RETURNING form for UPDATE statements. + diff --git a/doc/build/changelog/whatsnew_20.rst b/doc/build/changelog/whatsnew_20.rst index 36c3f53a74..d141ea7a07 100644 --- a/doc/build/changelog/whatsnew_20.rst +++ b/doc/build/changelog/whatsnew_20.rst @@ -1001,6 +1001,11 @@ get all drivers to this state: possible, usually with VALUES() - :ticket:`6047` * Emit a warning when RETURNING w/ executemany is used for non-supporting backend (currently no RETURNING backend has this limitation) - :ticket:`7907` +* The ORM :paramref:`_orm.Mapper.eager_defaults` parameter now defaults to a + a new setting ``"auto"``, which will enable "eager defaults" automatically + for INSERT statements, when the backend in use supports RETURNING with + "insertmanyvalues". See :ref:`orm_server_defaults` for documentation. + .. seealso:: diff --git a/doc/build/orm/persistence_techniques.rst b/doc/build/orm/persistence_techniques.rst index 601ddda521..911bc410db 100644 --- a/doc/build/orm/persistence_techniques.rst +++ b/doc/build/orm/persistence_techniques.rst @@ -264,10 +264,9 @@ generated automatically by the database are simple integer columns, which are implemented by the database as either a so-called "autoincrement" column, or from a sequence associated with the column. Every database dialect within SQLAlchemy Core supports a method of retrieving these primary key values which -is often native to the Python DBAPI, and in general this process is automatic, -with the exception of a database like Oracle that requires us to specify a -:class:`.Sequence` explicitly. There is more documentation regarding this -at :paramref:`_schema.Column.autoincrement`. +is often native to the Python DBAPI, and in general this process is automatic. +There is more documentation regarding this at +:paramref:`_schema.Column.autoincrement`. For server-generating columns that are not primary key columns or that are not simple autoincrementing integer columns, the ORM requires that these columns @@ -284,22 +283,29 @@ Case 1: non primary key, RETURNING or equivalent is supported ------------------------------------------------------------- In this case, columns should be marked as :class:`.FetchedValue` or with an -explicit :paramref:`_schema.Column.server_default`. The -:paramref:`_orm.Mapper.eager_defaults` parameter -may be used to indicate that these -columns should be fetched immediately upon INSERT and sometimes UPDATE:: +explicit :paramref:`_schema.Column.server_default`. The ORM will +automatically add these columns to the RETURNING clause when performing +INSERT statements, assuming the +:paramref:`_orm.Mapper.eager_defaults` parameter is set to ``True``, or +if left at its default setting of ``"auto"``, for dialects that support +both RETURNING as well as :ref:`insertmanyvalues `:: class MyModel(Base): __tablename__ = "my_table" id = mapped_column(Integer, primary_key=True) + + # server-side SQL date function generates a new timestamp timestamp = mapped_column(DateTime(), server_default=func.now()) - # assume a database trigger populates a value into this column - # during INSERT + # some other server-side function not named here, such as a trigger, + # populates a value into this column during INSERT special_identifier = mapped_column(String(50), server_default=FetchedValue()) + # set eager defaults to True. This is usually optional, as if the + # backend supports RETURNING + insertmanyvalues, eager defaults + # will take place regardless on INSERT __mapper_args__ = {"eager_defaults": True} Above, an INSERT statement that does not specify explicit values for @@ -312,12 +318,76 @@ above table will look like: INSERT INTO my_table DEFAULT VALUES RETURNING my_table.id, my_table.timestamp, my_table.special_identifier +.. versionchanged:: 2.0.0b5 The :paramref:`_orm.Mapper.eager_defaults` parameter now defaults + to a new setting ``"auto"``, which will automatically make use of RETURNING + to fetch server-generated default values on INSERT if the backing database + supports both RETURNING as well as :ref:`insertmanyvalues `. + +.. note:: The ``"auto"`` value for :paramref:`_orm.Mapper.eager_defaults` only + applies to INSERT statements. UPDATE statements will not use RETURNING, + even if available, unless :paramref:`_orm.Mapper.eager_defaults` is set to + ``True``. This is because there is no equivalent "insertmanyvalues" feature + for UPDATE, so UPDATE RETURNING will require that UPDATE statements are + emitted individually for each row being UPDATEd. + +Case 2: Table includes trigger-generated values which are not compatible with RETURNING +---------------------------------------------------------------------------------------- + +The ``"auto"`` setting of :paramref:`_orm.Mapper.eager_defaults` means that +a backend that supports RETURNING will usually make use of RETURNING with +INSERT statements in order to retreive newly generated default values. +However there are limitations of server-generated values that are generated +using triggers, such that RETURNING can't be used: + +* SQL Server does not allow RETURNING to be used in an INSERT statement + to retrieve a trigger-generated value; the statement will fail. + +* SQLite has limitations in combining the use of RETURNING with triggers, such + that the RETURNING clause will not have the INSERTed value available + +* Other backends may have limitations with RETURNING in conjunction with + triggers, or other kinds of server-generated values. + +To disable the use of RETURNING for such values, including not just for +server generated default values but also to ensure that the ORM will never +use RETURNING with a particular table, specify +:paramref:`_schema.Table.implicit_returning` +as ``False`` for the mapped :class:`.Table`. Using a Declarative mapping +this looks like:: + + class MyModel(Base): + __tablename__ = "my_table" + + id: Mapped[int] = mapped_column(primary_key=True) + data: Mapped[str] = mapped_column(String(50)) -Case 2: non primary key, RETURNING or equivalent is not supported or not needed + # assume a database trigger populates a value into this column + # during INSERT + special_identifier = mapped_column(String(50), server_default=FetchedValue()) + + # disable all use of RETURNING for the table + __table_args__ = {"implicit_returning": False} + +On SQL Server with the pyodbc driver, an INSERT for the above table will +not use RETURNING and will use the SQL Server ``scope_identity()`` function +to retreive the newly generated primary key value: + +.. sourcecode:: sql + + INSERT INTO my_table (data) VALUES (?); select scope_identity() + +.. seealso:: + + :ref:`mssql_insert_behavior` - background on the SQL Server dialect's + methods of fetching newly generated primary key values + +Case 3: non primary key, RETURNING or equivalent is not supported or not needed -------------------------------------------------------------------------------- -This case is the same as case 1 above, except we don't specify -:paramref:`.orm.mapper.eager_defaults`:: +This case is the same as case 1 above, except we typically don't want to +use :paramref:`.orm.Mapper.eager_defaults`, as its current implementation +in the absence of RETURNING support is to emit a SELECT-per-row, which +is not performant. Therefore the parameter is omitted in the mapping below:: class MyModel(Base): __tablename__ = "my_table" @@ -329,18 +399,21 @@ This case is the same as case 1 above, except we don't specify # during INSERT special_identifier = mapped_column(String(50), server_default=FetchedValue()) -After a record with the above mapping is INSERTed, the "timestamp" and -"special_identifier" columns will remain empty, and will be fetched via -a second SELECT statement when they are first accessed after the flush, e.g. -they are marked as "expired". - -If the :paramref:`.orm.mapper.eager_defaults` is still used, and the backend -database does not support RETURNING or an equivalent, the ORM will emit this -SELECT statement immediately following the INSERT statement. This is often -undesirable as it adds additional SELECT statements to the flush process that -may not be needed. Using the above mapping with the -:paramref:`.orm.mapper.eager_defaults` flag set to True against MySQL results -in SQL like this upon flush (minus the comment, which is for clarification only): +After a record with the above mapping is INSERTed on a backend that does not +include RETURNING or "insertmanyvalues" support, the "timestamp" and +"special_identifier" columns will remain empty, and will be fetched via a +second SELECT statement when they are first accessed after the flush, e.g. they +are marked as "expired". + +If the :paramref:`.orm.Mapper.eager_defaults` is explicitly provided with a +value of ``True``, and the backend database does not support RETURNING or an +equivalent, the ORM will emit a SELECT statement immediately following the +INSERT statement in order to fetch newly generated values; the ORM does not +currently have the ability to SELECT many newly inserted rows in batch if +RETURNING was not available. This is usually undesirable as it adds additional +SELECT statements to the flush process that may not be needed. Using the above +mapping with the :paramref:`.orm.Mapper.eager_defaults` flag set to True +against MySQL (not MariaDB) results in SQL like this upon flush: .. sourcecode:: sql @@ -350,71 +423,102 @@ in SQL like this upon flush (minus the comment, which is for clarification only) SELECT my_table.timestamp AS my_table_timestamp, my_table.special_identifier AS my_table_special_identifier FROM my_table WHERE my_table.id = %s -Case 3: primary key, RETURNING or equivalent is supported +A future release of SQLAlchemy may seek to improve the efficiency of +eager defaults in the abcense of RETURNING to batch many rows within a +single SELECT statement. + +Case 4: primary key, RETURNING or equivalent is supported ---------------------------------------------------------- A primary key column with a server-generated value must be fetched immediately upon INSERT; the ORM can only access rows for which it has a primary key value, -so if the primary key is generated by the server, the ORM needs a way for the -database to give us that new value immediately upon INSERT. +so if the primary key is generated by the server, the ORM needs a way +to retrieve that new value immediately upon INSERT. -As mentioned above, for integer "autoincrement" columns as well as +As mentioned above, for integer "autoincrement" columns, as well as +columns marked with :class:`.Identity` and special constructs such as PostgreSQL SERIAL, these types are handled automatically by the Core; databases include functions for fetching the "last inserted id" where RETURNING is not supported, and where RETURNING is supported SQLAlchemy will use that. -However, for non-integer values, as well as for integer values that must be -explicitly linked to a sequence or other triggered routine, the server default -generation must be marked in the table metadata. - -For an explicit sequence as we use with Oracle, this just means we are using -the :class:`.Sequence` construct:: +For example, using Oracle with a column marked as :class:`.Identity`, +RETURNING is used automatically to fetch the new primary key value:: class MyOracleModel(Base): __tablename__ = "my_table" - id = mapped_column(Integer, Sequence("my_sequence", start=1), primary_key=True) - data = mapped_column(String(50)) + id: Mapped[int] = mapped_column(Identity(), primary_key=True) + data: Mapped[str] = mapped_column(String(50)) The INSERT for a model as above on Oracle looks like: .. sourcecode:: sql - INSERT INTO my_table (id, data) VALUES (my_sequence.nextval, :data) RETURNING my_table.id INTO :ret_0 + INSERT INTO my_table (data) VALUES (:data) RETURNING my_table.id INTO :ret_0 -Where above, SQLAlchemy renders ``my_sequence.nextval`` for the primary key column -and also uses RETURNING to get the new value back immediately. +SQLAlchemy renders an INSERT for the "data" field, but only includes "id" in +the RETURNING clause, so that server-side generation for "id" will take +place and the new value will be returned immediately. -For datatypes that generate values automatically, or columns that are populated -by a trigger, we use :class:`.FetchedValue`. Below is a model that uses a -SQL Server TIMESTAMP column as the primary key, which generates values automatically:: +For non-integer values generated by server side functions or triggers, as well +as for integer values that come from constructs outside the table itself, +including explicit sequences and triggers, the server default generation must +be marked in the table metadata. Using Oracle as the example again, we can +illustrate a similar table as above naming an explicit sequence using the +:class:`.Sequence` construct:: - class MyModel(Base): + class MyOracleModel(Base): __tablename__ = "my_table" - timestamp = mapped_column( + id: Mapped[int] = mapped_column(Sequence("my_oracle_seq"), primary_key=True) + data: Mapped[str] = mapped_column(String(50)) + +An INSERT for this version of the model on Oracle would look like: + +.. sourcecode:: sql + + INSERT INTO my_table (id, data) VALUES (my_oracle_seq.nextval, :data) RETURNING my_table.id INTO :ret_0 + +Where above, SQLAlchemy renders ``my_sequence.nextval`` for the primary key +column so that it is used for new primary key generation, and also uses +RETURNING to get the new value back immediately. + +If the source of data is not represented by a simple SQL function or +:class:`.Sequence`, such as when using triggers or database-specific datatypes +that produce new values, the presence of a value-generating default may be +indicated by using :class:`.FetchedValue` within the column definition. Below +is a model that uses a SQL Server TIMESTAMP column as the primary key; on SQL +Server, this datatype generates new values automatically, so this is indicated +in the table metadata by indicating :class:`.FetchedValue` for the +:paramref:`.Column.server_default` parameter:: + + class MySQLServerModel(Base): + __tablename__ = "my_table" + + timestamp: Mapped[datetime.datetime] = mapped_column( TIMESTAMP(), server_default=FetchedValue(), primary_key=True ) + data: Mapped[str] = mapped_column(String(50)) An INSERT for the above table on SQL Server looks like: .. sourcecode:: sql - INSERT INTO my_table OUTPUT inserted.timestamp DEFAULT VALUES + INSERT INTO my_table (data) OUTPUT inserted.timestamp VALUES (?) -Case 4: primary key, RETURNING or equivalent is not supported +Case 5: primary key, RETURNING or equivalent is not supported -------------------------------------------------------------- -In this area we are generating rows for a database such as SQLite or MySQL +In this area we are generating rows for a database such as MySQL where some means of generating a default is occurring on the server, but is outside of the database's usual autoincrement routine. In this case, we have to make sure SQLAlchemy can "pre-execute" the default, which means it has to be an explicit SQL expression. .. note:: This section will illustrate multiple recipes involving - datetime values for MySQL and SQLite, since the datetime datatypes on these - two backends have additional idiosyncratic requirements that are useful to - illustrate. Keep in mind however that SQLite and MySQL require an explicit + datetime values for MySQL, since the datetime datatypes on this + backend has additional idiosyncratic requirements that are useful to + illustrate. Keep in mind however that MySQL requires an explicit "pre-executed" default generator for *any* auto-generated datatype used as the primary key other than the usual single-column autoincrementing integer value. @@ -471,38 +575,6 @@ INSERT looks like: INSERT INTO my_table (timestamp) VALUES (%s) (b'2018-08-09 13:08:46',) -SQLite with DateTime primary key -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -For SQLite, new timestamps can be generated using the SQL function -``datetime('now', 'localtime')`` (or specify ``'utc'`` for UTC), -however making things more complicated is that this returns a string -value, which is then incompatible with SQLAlchemy's :class:`.DateTime` -datatype (even though the datatype converts the information back into a -string for the SQLite backend, it must be passed through as a Python datetime). -We therefore must also specify that we'd like to coerce the return value to -:class:`.DateTime` when it is returned from the function, which we achieve -by passing this as the ``type_`` parameter:: - - class MyModel(Base): - __tablename__ = "my_table" - - timestamp = mapped_column( - DateTime, - default=func.datetime("now", "localtime", type_=DateTime), - primary_key=True, - ) - -The above mapping upon INSERT will look like: - -.. sourcecode:: sql - - SELECT datetime(?, ?) AS datetime_1 - ('now', 'localtime') - INSERT INTO my_table (timestamp) VALUES (?) - ('2018-10-02 13:37:33.000000',) - - .. seealso:: :ref:`metadata_defaults_toplevel` @@ -521,8 +593,8 @@ are set up using the :paramref:`_schema.Column.default` and These SQL expressions currently are subject to the same limitations within the ORM as occurs for true server-side defaults; they won't be eagerly fetched with -RETURNING when using :paramref:`_orm.Mapper.eager_defaults` unless the -:class:`.FetchedValue` directive is associated with the +RETURNING when :paramref:`_orm.Mapper.eager_defaults` is set to ``"auto"`` or +``True`` unless the :class:`.FetchedValue` directive is associated with the :class:`_schema.Column`, even though these expressions are not DDL server defaults and are actively rendered by SQLAlchemy itself. This limitation may be addressed in future SQLAlchemy releases. diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py index a0049c361e..aa640727fa 100644 --- a/lib/sqlalchemy/dialects/mssql/base.py +++ b/lib/sqlalchemy/dialects/mssql/base.py @@ -232,6 +232,7 @@ integer values in Python 3), use :class:`_types.TypeDecorator` as follows:: ) name = Column(String) +.. _mssql_insert_behavior: INSERT behavior ^^^^^^^^^^^^^^^^ diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 7a75246212..90c463ce62 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -230,7 +230,7 @@ class Mapper( passive_updates: bool = True, passive_deletes: bool = False, confirm_deleted_rows: bool = True, - eager_defaults: bool = False, + eager_defaults: Literal[True, False, "auto"] = "auto", legacy_is_orphan: bool = False, _compiled_cache_size: int = 100, ): @@ -336,14 +336,30 @@ class Mapper( value of server-generated default values after an INSERT or UPDATE, rather than leaving them as expired to be fetched on next access. This can be used for event schemes where the server-generated values - are needed immediately before the flush completes. By default, - this scheme will emit an individual ``SELECT`` statement per row - inserted or updated, which note can add significant performance - overhead. However, if the - target database supports :term:`RETURNING`, the default values will - be returned inline with the INSERT or UPDATE statement, which can - greatly enhance performance for an application that needs frequent - access to just-generated server defaults. + are needed immediately before the flush completes. + + The fetch of values occurs either by using ``RETURNING`` inline + with the ``INSERT`` or ``UPDATE`` statement, or by adding an + additional ``SELECT`` statement subsequent to the ``INSERT`` or + ``UPDATE``, if the backend does not support ``RETURNING``. + + The use of ``RETURNING`` is extremely performant in particular for + ``INSERT`` statements where SQLAlchemy can take advantage of + :ref:`insertmanyvalues `, whereas the use of + an additional ``SELECT`` is relatively poor performing, adding + additional SQL round trips which would be unnecessary if these new + attributes are not to be accessed in any case. + + For this reason, :paramref:`.Mapper.eager_defaults` defaults to the + string value ``"auto"``, which indicates that server defaults for + INSERT should be fetched using ``RETURNING`` if the backing database + supports it and if the dialect in use supports "insertmanyreturning" + for an INSERT statement. If the backing database does not support + ``RETURNING`` or "insertmanyreturning" is not available, server + defaults will not be fetched. + + .. versionchanged:: 2.0.0b5 added the "auto" option for + :paramref:`.Mapper.eager_defaults` .. seealso:: @@ -352,6 +368,12 @@ class Mapper( .. versionchanged:: 0.9.0 The ``eager_defaults`` option can now make use of :term:`RETURNING` for backends which support it. + .. versionchanged:: 2.0.0 RETURNING now works with multiple rows + INSERTed at once using the + :ref:`insertmanyvalues ` feature, which + among other things allows the :paramref:`.Mapper.eager_defaults` + feature to be very performant on supporting backends. + :param exclude_properties: A list or set of string column names to be excluded from mapping. @@ -818,6 +840,18 @@ class Mapper( self._log("constructed") self._expire_memoizations() + def _prefer_eager_defaults(self, dialect, table): + if self.eager_defaults == "auto": + if not table.implicit_returning: + return False + + return ( + table in self._server_default_col_keys + and dialect.insert_executemany_returning + ) + else: + return self.eager_defaults + def _gen_cache_key(self, anon_map, bindparams): return (self,) diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index c236ad1cf7..0eff4e1fa0 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -381,7 +381,9 @@ def _collect_insert_commands( # compare to pk_keys_by_table has_all_pks = mapper._pk_keys_by_table[table].issubset(params) - if mapper.base_mapper.eager_defaults: + if mapper.base_mapper._prefer_eager_defaults( + connection.dialect, table + ): has_all_defaults = mapper._server_default_col_keys[ table ].issubset(params) @@ -491,7 +493,7 @@ def _collect_update_commands( ): params[col.key] = value - if mapper.base_mapper.eager_defaults: + if mapper.base_mapper.eager_defaults is True: has_all_defaults = ( mapper._server_onupdate_default_col_keys[table] ).issubset(params) @@ -787,7 +789,12 @@ def _emit_update_statements( if ( bookkeeping and not has_all_defaults - and mapper.base_mapper.eager_defaults + and mapper.base_mapper.eager_defaults is True + # change as of #8889 - if RETURNING is not going to be used anyway, + # (applies to MySQL, MariaDB which lack UPDATE RETURNING) ensure + # we can do an executemany UPDATE which is more efficient + and table.implicit_returning + and connection.dialect.update_returning ): statement = statement.return_defaults( *mapper._server_onupdate_default_cols[table] @@ -808,7 +815,11 @@ def _emit_update_statements( assert_singlerow and connection.dialect.supports_sane_multi_rowcount ) - allow_multirow = has_all_defaults and not needs_version_id + + # change as of #8889 - if RETURNING is not going to be used anyway, + # (applies to MySQL, MariaDB which lack UPDATE RETURNING) ensure + # we can do an executemany UPDATE which is more efficient + allow_executemany = not return_defaults and not needs_version_id if hasvalue: for ( @@ -842,7 +853,7 @@ def _emit_update_statements( rows += c.rowcount check_rowcount = assert_singlerow else: - if not allow_multirow: + if not allow_executemany: check_rowcount = assert_singlerow for ( state, @@ -991,7 +1002,9 @@ def _emit_insert_statements( not bookkeeping or ( has_all_defaults - or not base_mapper.eager_defaults + or not base_mapper._prefer_eager_defaults( + connection.dialect, table + ) or not table.implicit_returning or not connection.dialect.insert_returning ) @@ -1067,7 +1080,9 @@ def _emit_insert_statements( else: do_executemany = False - if not has_all_defaults and base_mapper.eager_defaults: + if not has_all_defaults and base_mapper._prefer_eager_defaults( + connection.dialect, table + ): statement = statement.return_defaults( *mapper._server_default_cols[table] ) @@ -1282,9 +1297,9 @@ def _emit_post_update_statements( assert_singlerow and connection.dialect.supports_sane_multi_rowcount ) - allow_multirow = not needs_version_id or assert_multirow + allow_executemany = not needs_version_id or assert_multirow - if not allow_multirow: + if not allow_executemany: check_rowcount = assert_singlerow for state, state_dict, mapper_rec, connection, params in records: @@ -1475,7 +1490,9 @@ def _finalize_insert_update_commands(base_mapper, uowtransaction, states): # it isn't expired. toload_now = [] - if base_mapper.eager_defaults: + # this is specifically to emit a second SELECT for eager_defaults, + # so only if it's set to True, not "auto" + if base_mapper.eager_defaults is True: toload_now.extend( state._unloaded_non_object.intersection( mapper._server_default_plus_onupdate_propkeys diff --git a/lib/sqlalchemy/testing/__init__.py b/lib/sqlalchemy/testing/__init__.py index 993fc4954f..6454750f53 100644 --- a/lib/sqlalchemy/testing/__init__.py +++ b/lib/sqlalchemy/testing/__init__.py @@ -56,6 +56,7 @@ from .config import requirements as requires from .config import skip_test from .config import Variation from .config import variation +from .config import variation_fixture from .exclusions import _is_excluded from .exclusions import _server_version from .exclusions import against as _against diff --git a/lib/sqlalchemy/testing/config.py b/lib/sqlalchemy/testing/config.py index 6adcf5b640..b444eb39f9 100644 --- a/lib/sqlalchemy/testing/config.py +++ b/lib/sqlalchemy/testing/config.py @@ -157,9 +157,33 @@ class Variation: def __str__(self): return f"{self._argname}={self._name!r}" + def __repr__(self): + return str(self) + def fail(self) -> NoReturn: fail(f"Unknown {self}") + @classmethod + def idfn(cls, variation): + return variation.name + + @classmethod + def generate_cases(cls, argname, cases): + case_names = [ + argname if c is True else "not_" + argname if c is False else c + for c in cases + ] + + typ = type( + argname, + (Variation,), + { + "__slots__": tuple(case_names), + }, + ) + + return [typ(casename, argname, case_names) for casename in case_names] + def variation(argname, cases): """a helper around testing.combinations that provides a single namespace @@ -203,26 +227,17 @@ def variation(argname, cases): else (entry, None) for entry in cases ] - case_names = [ - argname if c is True else "not_" + argname if c is False else c - for c, l in cases_plus_limitations - ] - typ = type( - argname, - (Variation,), - { - "__slots__": tuple(case_names), - }, + variations = Variation.generate_cases( + argname, [c for c, l in cases_plus_limitations] ) - return combinations( *[ - (casename, typ(casename, argname, case_names), limitation) + (variation._name, variation, limitation) if limitation is not None - else (casename, typ(casename, argname, case_names)) - for casename, (case, limitation) in zip( - case_names, cases_plus_limitations + else (variation._name, variation) + for variation, (case, limitation) in zip( + variations, cases_plus_limitations ) ], id_="ia", @@ -230,6 +245,14 @@ def variation(argname, cases): ) +def variation_fixture(argname, cases, scope="function"): + return fixture( + params=Variation.generate_cases(argname, cases), + ids=Variation.idfn, + scope=scope, + ) + + def fixture(*arg: Any, **kw: Any) -> Any: return _fixture_functions.fixture(*arg, **kw) diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index 0ba9007988..5803d51bc0 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -3074,9 +3074,15 @@ class OptimizedLoadTest(fixtures.MappedTest): eq_(s1test.comp, Comp("ham", "cheese")) eq_(s2test.comp, Comp("bacon", "eggs")) - def test_load_expired_on_pending(self): + @testing.variation("eager_defaults", [True, False]) + def test_load_expired_on_pending(self, eager_defaults): base, sub = self.tables.base, self.tables.sub + expected_eager_defaults = bool(eager_defaults) + expect_returning = ( + expected_eager_defaults and testing.db.dialect.insert_returning + ) + class Base(fixtures.BasicEntity): pass @@ -3084,7 +3090,11 @@ class OptimizedLoadTest(fixtures.MappedTest): pass self.mapper_registry.map_imperatively( - Base, base, polymorphic_on=base.c.type, polymorphic_identity="base" + Base, + base, + polymorphic_on=base.c.type, + polymorphic_identity="base", + eager_defaults=bool(eager_defaults), ) self.mapper_registry.map_imperatively( Sub, sub, inherits=Base, polymorphic_identity="sub" @@ -3095,13 +3105,30 @@ class OptimizedLoadTest(fixtures.MappedTest): self.assert_sql_execution( testing.db, sess.flush, - CompiledSQL( - "INSERT INTO base (data, type) VALUES (:data, :type)", - [{"data": "s1", "type": "sub"}], - ), - CompiledSQL( - "INSERT INTO sub (id, sub) VALUES (:id, :sub)", - lambda ctx: {"id": s1.id, "sub": None}, + Conditional( + expect_returning, + [ + CompiledSQL( + "INSERT INTO base (data, type) VALUES (:data, :type) " + "RETURNING base.id, base.counter", + [{"data": "s1", "type": "sub"}], + ), + CompiledSQL( + "INSERT INTO sub (id, sub) VALUES (:id, :sub) " + "RETURNING sub.subcounter, sub.subcounter2", + lambda ctx: {"id": s1.id, "sub": None}, + ), + ], + [ + CompiledSQL( + "INSERT INTO base (data, type) VALUES (:data, :type)", + [{"data": "s1", "type": "sub"}], + ), + CompiledSQL( + "INSERT INTO sub (id, sub) VALUES (:id, :sub)", + lambda ctx: {"id": s1.id, "sub": None}, + ), + ], ), ) @@ -3111,12 +3138,19 @@ class OptimizedLoadTest(fixtures.MappedTest): self.assert_sql_execution( testing.db, go, - CompiledSQL( - "SELECT base.counter AS base_counter, " - "sub.subcounter AS sub_subcounter, " - "sub.subcounter2 AS sub_subcounter2 FROM base JOIN sub " - "ON base.id = sub.id WHERE base.id = :pk_1", - lambda ctx: {"pk_1": s1.id}, + Conditional( + expect_returning, + [], + [ + CompiledSQL( + "SELECT base.counter AS base_counter, " + "sub.subcounter AS sub_subcounter, " + "sub.subcounter2 AS sub_subcounter2 " + "FROM base JOIN sub " + "ON base.id = sub.id WHERE base.id = :pk_1", + lambda ctx: {"pk_1": s1.id}, + ), + ], ), ) diff --git a/test/orm/test_defaults.py b/test/orm/test_defaults.py index e738689b89..fb6fba704a 100644 --- a/test/orm/test_defaults.py +++ b/test/orm/test_defaults.py @@ -17,6 +17,7 @@ from sqlalchemy.testing.schema import Table class TriggerDefaultsTest(fixtures.MappedTest): __requires__ = ("row_triggers",) + __backend__ = True @classmethod def define_tables(cls, metadata): @@ -39,6 +40,7 @@ class TriggerDefaultsTest(fixtures.MappedTest): sa.schema.FetchedValue(), sa.schema.FetchedValue(for_update=True), ), + implicit_returning=False, ) dialect_name = testing.db.dialect.name @@ -382,12 +384,7 @@ class ComputedDefaultsOnUpdateTest(fixtures.MappedTest): asserter.assert_( CompiledSQL( "UPDATE test SET foo=:foo WHERE test.id = :test_id", - [{"foo": 5, "test_id": 1}], - enable_returning=False, - ), - CompiledSQL( - "UPDATE test SET foo=:foo WHERE test.id = :test_id", - [{"foo": 6, "test_id": 2}], + [{"foo": 5, "test_id": 1}, {"foo": 6, "test_id": 2}], enable_returning=False, ), CompiledSQL( diff --git a/test/orm/test_expire.py b/test/orm/test_expire.py index a5fd7533e9..f851f3698c 100644 --- a/test/orm/test_expire.py +++ b/test/orm/test_expire.py @@ -1826,7 +1826,9 @@ class LifecycleTest(fixtures.MappedTest): def setup_mappers(cls): cls.mapper_registry.map_imperatively(cls.classes.Data, cls.tables.data) cls.mapper_registry.map_imperatively( - cls.classes.DataFetched, cls.tables.data_fetched + cls.classes.DataFetched, + cls.tables.data_fetched, + eager_defaults=False, ) cls.mapper_registry.map_imperatively( cls.classes.DataDefer, @@ -1886,7 +1888,6 @@ class LifecycleTest(fixtures.MappedTest): def go(): eq_(d1.data, None) - # this one is marked as "fetch" so we emit SQL self.assert_sql_count(testing.db, go, 1) def test_cols_missing_in_load(self): diff --git a/test/orm/test_unitofwork.py b/test/orm/test_unitofwork.py index 79d4adacf2..5835ef65a2 100644 --- a/test/orm/test_unitofwork.py +++ b/test/orm/test_unitofwork.py @@ -1135,7 +1135,8 @@ class DefaultTest(fixtures.MappedTest): class Secondary(cls.Comparable): pass - def test_insert(self): + @testing.variation("eager_defaults", ["auto", True, False]) + def test_insert(self, eager_defaults): althohoval, hohoval, default_t, Hoho = ( self.other.althohoval, self.other.hohoval, @@ -1143,7 +1144,13 @@ class DefaultTest(fixtures.MappedTest): self.classes.Hoho, ) - self.mapper_registry.map_imperatively(Hoho, default_t) + mp = self.mapper_registry.map_imperatively( + Hoho, + default_t, + eager_defaults="auto" + if eager_defaults.auto + else bool(eager_defaults), + ) h1 = Hoho(hoho=althohoval) h2 = Hoho(counter=12) @@ -1162,12 +1169,18 @@ class DefaultTest(fixtures.MappedTest): # test deferred load of attributes, one select per instance self.assert_(h2.hoho == h4.hoho == h5.hoho == hohoval) - self.sql_count_(3, go) + if mp._prefer_eager_defaults(testing.db.dialect, default_t): + self.sql_count_(0, go) + else: + self.sql_count_(3, go) def go(): self.assert_(h1.counter == h4.counter == h5.counter == 7) - self.sql_count_(1, go) + if mp._prefer_eager_defaults(testing.db.dialect, default_t): + self.sql_count_(0, go) + else: + self.sql_count_(1, go) def go(): self.assert_(h3.counter == h2.counter == 12) diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index 468d43063d..ae47dfa4fd 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -34,6 +34,7 @@ from sqlalchemy.testing import engines from sqlalchemy.testing import eq_ from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ +from sqlalchemy.testing import variation_fixture from sqlalchemy.testing.assertsql import AllOf from sqlalchemy.testing.assertsql import CompiledSQL from sqlalchemy.testing.assertsql import Conditional @@ -2077,7 +2078,7 @@ class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults): class T(fixtures.ComparableEntity): pass - self.mapper_registry.map_imperatively(T, t) + mp = self.mapper_registry.map_imperatively(T, t) sess = fixture_session() sess.add_all( [ @@ -2095,6 +2096,17 @@ class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults): ] ) + eager_defaults = mp._prefer_eager_defaults( + testing.db.dialect, mp.local_table + ) + + if eager_defaults: + tdef_col = ", t.def_" + tdef_returning = " RETURNING t.def_" + else: + tdef_col = "" + tdef_returning = "" + self.assert_sql_execution( testing.db, sess.flush, @@ -2102,7 +2114,8 @@ class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults): testing.db.dialect.insert_executemany_returning, [ CompiledSQL( - "INSERT INTO t (data) VALUES (:data) RETURNING t.id", + f"INSERT INTO t (data) VALUES (:data) " + f"RETURNING t.id{tdef_col}", [{"data": "t1"}, {"data": "t2"}], ), ], @@ -2116,7 +2129,8 @@ class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults): ], ), CompiledSQL( - "INSERT INTO t (id, data) VALUES (:id, :data)", + f"INSERT INTO t (id, data) " + f"VALUES (:id, :data){tdef_returning}", [ {"data": "t3", "id": 3}, {"data": "t4", "id": 4}, @@ -2124,11 +2138,13 @@ class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults): ], ), CompiledSQL( - "INSERT INTO t (id, data) VALUES (:id, lower(:lower_1))", + f"INSERT INTO t (id, data) " + f"VALUES (:id, lower(:lower_1)){tdef_returning}", {"lower_1": "t6", "id": 6}, ), CompiledSQL( - "INSERT INTO t (id, data) VALUES (:id, :data)", + f"INSERT INTO t (id, data) " + f"VALUES (:id, :data){tdef_returning}", [{"data": "t7", "id": 7}, {"data": "t8", "id": 8}], ), CompiledSQL( @@ -2139,7 +2155,8 @@ class BatchInsertsTest(fixtures.MappedTest, testing.AssertsExecutionResults): ], ), CompiledSQL( - "INSERT INTO t (id, data) VALUES (:id, :data)", + f"INSERT INTO t (id, data) " + f"VALUES (:id, :data){tdef_returning}", {"data": "t11", "id": 11}, ), ) @@ -2385,30 +2402,30 @@ class EagerDefaultsTest(fixtures.MappedTest): class Thing4(cls.Basic): pass - @classmethod - def setup_mappers(cls): - Thing = cls.classes.Thing + def setup_mappers(self): + eager_defaults = True + Thing = self.classes.Thing - cls.mapper_registry.map_imperatively( - Thing, cls.tables.test, eager_defaults=True + self.mapper_registry.map_imperatively( + Thing, self.tables.test, eager_defaults=eager_defaults ) - Thing2 = cls.classes.Thing2 + Thing2 = self.classes.Thing2 - cls.mapper_registry.map_imperatively( - Thing2, cls.tables.test2, eager_defaults=True + self.mapper_registry.map_imperatively( + Thing2, self.tables.test2, eager_defaults=eager_defaults ) - Thing3 = cls.classes.Thing3 + Thing3 = self.classes.Thing3 - cls.mapper_registry.map_imperatively( - Thing3, cls.tables.test3, eager_defaults=True + self.mapper_registry.map_imperatively( + Thing3, self.tables.test3, eager_defaults=eager_defaults ) - Thing4 = cls.classes.Thing4 + Thing4 = self.classes.Thing4 - cls.mapper_registry.map_imperatively( - Thing4, cls.tables.test4, eager_defaults=True + self.mapper_registry.map_imperatively( + Thing4, self.tables.test4, eager_defaults=eager_defaults ) def test_server_insert_defaults_present(self): @@ -3111,6 +3128,218 @@ class EagerDefaultsTest(fixtures.MappedTest): ) +class EagerDefaultsSettingTest( + testing.AssertsExecutionResults, fixtures.TestBase +): + __backend__ = True + + @variation_fixture("eager_defaults", ["unspecified", "auto", True, False]) + def eager_defaults_variations(self, request): + yield request.param + + @variation_fixture("implicit_returning", [True, False]) + def implicit_returning_variations(self, request): + yield request.param + + @testing.fixture + def define_tables( + self, metadata, connection, implicit_returning_variations + ): + implicit_returning = bool(implicit_returning_variations) + + t = Table( + "test", + metadata, + Column("id", Integer, primary_key=True), + Column( + "foo", + Integer, + server_default="3", + ), + Column("bar", Integer, server_onupdate=FetchedValue()), + implicit_returning=implicit_returning, + ) + metadata.create_all(connection) + return t + + @testing.fixture + def setup_mappers( + self, define_tables, eager_defaults_variations, registry + ): + class Thing: + pass + + if eager_defaults_variations.unspecified: + registry.map_imperatively(Thing, define_tables) + else: + eager_defaults = ( + "auto" + if eager_defaults_variations.auto + else bool(eager_defaults_variations) + ) + registry.map_imperatively( + Thing, define_tables, eager_defaults=eager_defaults + ) + return Thing + + def test_eager_default_setting_inserts( + self, + setup_mappers, + eager_defaults_variations, + implicit_returning_variations, + connection, + ): + Thing = setup_mappers + s = Session(connection) + + t1, t2 = (Thing(id=1, bar=6), Thing(id=2, bar=6)) + + s.add_all([t1, t2]) + + expected_eager_defaults = eager_defaults_variations.eager_defaults or ( + ( + eager_defaults_variations.auto + or eager_defaults_variations.unspecified + ) + and connection.dialect.insert_executemany_returning + and bool(implicit_returning_variations) + ) + expect_returning = ( + expected_eager_defaults + and connection.dialect.insert_returning + and bool(implicit_returning_variations) + ) + + with self.sql_execution_asserter(connection) as asserter: + s.flush() + + asserter.assert_( + Conditional( + expect_returning, + [ + CompiledSQL( + "INSERT INTO test (id, bar) VALUES (:id, :bar) " + "RETURNING test.foo", + [ + {"id": 1, "bar": 6}, + {"id": 2, "bar": 6}, + ], + ) + ], + [ + CompiledSQL( + "INSERT INTO test (id, bar) VALUES (:id, :bar)", + [ + {"id": 1, "bar": 6}, + {"id": 2, "bar": 6}, + ], + ), + Conditional( + expected_eager_defaults and not expect_returning, + [ + CompiledSQL( + "SELECT test.foo AS test_foo " + "FROM test WHERE test.id = :pk_1", + [{"pk_1": 1}], + ), + CompiledSQL( + "SELECT test.foo AS test_foo " + "FROM test WHERE test.id = :pk_1", + [{"pk_1": 2}], + ), + ], + [], + ), + ], + ) + ) + + def test_eager_default_setting_updates( + self, + setup_mappers, + eager_defaults_variations, + implicit_returning_variations, + connection, + ): + Thing = setup_mappers + s = Session(connection) + + t1, t2 = (Thing(id=1, foo=5), Thing(id=2, foo=5)) + + s.add_all([t1, t2]) + s.flush() + + expected_eager_defaults = eager_defaults_variations.eager_defaults + expect_returning = ( + expected_eager_defaults + and connection.dialect.update_returning + and bool(implicit_returning_variations) + ) + + t1.foo = 7 + t2.foo = 12 + + with self.sql_execution_asserter(connection) as asserter: + s.flush() + + asserter.assert_( + Conditional( + expect_returning, + [ + CompiledSQL( + "UPDATE test SET foo=:foo WHERE test.id = :test_id " + "RETURNING test.bar", + [ + {"test_id": 1, "foo": 7}, + ], + ), + CompiledSQL( + "UPDATE test SET foo=:foo WHERE test.id = :test_id " + "RETURNING test.bar", + [ + {"test_id": 2, "foo": 12}, + ], + ), + ], + [ + Conditional( + expected_eager_defaults and not expect_returning, + [ + CompiledSQL( + "UPDATE test SET foo=:foo " + "WHERE test.id = :test_id", + [ + {"test_id": 1, "foo": 7}, + {"test_id": 2, "foo": 12}, + ], + ), + CompiledSQL( + "SELECT test.bar AS test_bar " + "FROM test WHERE test.id = :pk_1", + [{"pk_1": 1}], + ), + CompiledSQL( + "SELECT test.bar AS test_bar " + "FROM test WHERE test.id = :pk_1", + [{"pk_1": 2}], + ), + ], + [ + CompiledSQL( + "UPDATE test SET foo=:foo " + "WHERE test.id = :test_id", + [ + {"test_id": 1, "foo": 7}, + {"test_id": 2, "foo": 12}, + ], + ), + ], + ), + ], + ) + ) + + class TypeWoBoolTest(fixtures.MappedTest, testing.AssertsExecutionResults): """test support for custom datatypes that return a non-__bool__ value when compared via __eq__(), eg. ticket 3469""" -- 2.47.2