Remove errant assertion from unit of work

Fixed ORM unit of work regression where an errant "assert primary_key"
statement interferes with primary key generation sequences that don't
actually consider the columns in the table to use a real primary key
constraint, instead using :paramref:`_orm.mapper.primary_key` to establish
certain columns as "primary".

Also remove errant "identity" requirement which does not seem to
represent any current backend and is applied to
test/sql/test_defaults.py->AutoIncrementTest, but these tests work
on all backends.

Fixes: #5867
Change-Id: I4502ca5079d824d7b4d055194947aa1a00effde7

diff --git a/doc/build/changelog/unreleased_14/5867.rst b/doc/build/changelog/unreleased_14/5867.rst
new file mode 100644 (file)
index 0000000..739377e
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/5867.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, regression, orm
+    :tickets: 5867
+
+    Fixed ORM unit of work regression where an errant "assert primary_key"
+    statement interferes with primary key generation sequences that don't
+    actually consider the columns in the table to use a real primary key
+    constraint, instead using :paramref:`_orm.mapper.primary_key` to establish
+    certain columns as "primary".

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index b57963eb3f3d11be09d581dcf4d219c714cd7c8f..42b7875802fa5262e11ff582fda2e9f1c4504c4b 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -1222,7 +1222,6 @@ def _emit_insert_statements(
                         )
 
                     primary_key = result.inserted_primary_key
-                    assert primary_key
                     for pk, col in zip(
                         primary_key, mapper._pks_by_table[table]
                     ):

diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index 65089f773ce1c3d8e46be04e63c485b11d91fe5c..822b47632ee0cf0050ef17fa5ba75bb17647776f 100644 (file)
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -1,20 +1,25 @@
 from sqlalchemy import cast
+from sqlalchemy import DateTime
 from sqlalchemy import event
 from sqlalchemy import exc
 from sqlalchemy import FetchedValue
 from sqlalchemy import ForeignKey
 from sqlalchemy import func
+from sqlalchemy import Identity
 from sqlalchemy import inspect
 from sqlalchemy import Integer
 from sqlalchemy import JSON
 from sqlalchemy import literal
 from sqlalchemy import select
+from sqlalchemy import Sequence
 from sqlalchemy import String
 from sqlalchemy import testing
 from sqlalchemy import text
 from sqlalchemy import util
+from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import attributes
 from sqlalchemy.orm import backref
+from sqlalchemy.orm import clear_mappers
 from sqlalchemy.orm import exc as orm_exc
 from sqlalchemy.orm import mapper
 from sqlalchemy.orm import relationship
@@ -3098,3 +3103,80 @@ class EnsureCacheTest(fixtures.FutureEngineMixin, UOWTest):
 
             is_(conn._execution_options["compiled_cache"], cache)
             eq_(len(inspect(User)._compiled_cache), 3)
+
+
+class ORMOnlyPrimaryKeyTest(fixtures.TestBase):
+    @testing.requires.identity_columns
+    @testing.requires.returning
+    def test_a(self, base, run_test):
+        class A(base):
+            __tablename__ = "a"
+
+            id = Column(Integer, Identity())
+            included_col = Column(Integer)
+
+            __mapper_args__ = {"primary_key": [id], "eager_defaults": True}
+
+        run_test(A, A())
+
+    @testing.requires.sequences_as_server_defaults
+    @testing.requires.returning
+    def test_b(self, base, run_test):
+
+        seq = Sequence("x_seq")
+
+        class A(base):
+            __tablename__ = "a"
+
+            id = Column(Integer, seq, server_default=seq.next_value())
+            included_col = Column(Integer)
+
+            __mapper_args__ = {"primary_key": [id], "eager_defaults": True}
+
+        run_test(A, A())
+
+    def test_c(self, base, run_test):
+        class A(base):
+            __tablename__ = "a"
+
+            id = Column(Integer, nullable=False)
+            included_col = Column(Integer)
+
+            __mapper_args__ = {"primary_key": [id]}
+
+        a1 = A(id=1, included_col=select(1).scalar_subquery())
+        run_test(A, a1)
+
+    def test_d(self, base, run_test):
+        class A(base):
+            __tablename__ = "a"
+
+            id = Column(Integer, nullable=False)
+            updated_at = Column(DateTime, server_default=func.now())
+
+            __mapper_args__ = {"primary_key": [id], "eager_defaults": True}
+
+        a1 = A(id=1)
+        run_test(A, a1)
+
+    @testing.fixture
+    def base(self, metadata):
+        yield declarative_base(metadata=metadata)
+        clear_mappers()
+
+    @testing.fixture
+    def run_test(self, metadata, connection):
+        def go(A, a1):
+            metadata.create_all(connection)
+
+            with Session(connection) as s:
+                s.add(a1)
+
+                s.flush()
+                eq_(a1.id, 1)
+                s.commit()
+
+                aa = s.query(A).first()
+                is_(a1, aa)
+
+        return go

diff --git a/test/requirements.py b/test/requirements.py
index 3c9b39ac71567da78d61f90641fb0aaf8fa0089f..1a4c5f646d066bd8c17775bf314b5705e577e3ea 100644 (file)
--- a/test/requirements.py
+++ b/test/requirements.py
@@ -252,21 +252,6 @@ class DefaultRequirements(SuiteRequirements):
 
         return skip_if(["oracle"])
 
-    @property
-    def identity(self):
-        """Target database must support GENERATED AS IDENTITY or a facsimile.
-
-        Includes GENERATED AS IDENTITY, AUTOINCREMENT, AUTO_INCREMENT, or other
-        column DDL feature that fills in a DB-generated identifier at
-        INSERT-time without requiring pre-execution of a SEQUENCE or other
-        artifact.
-
-        """
-        return skip_if(
-            ["firebird", "oracle", "postgresql", "sybase"],
-            "not supported by database",
-        )
-
     @property
     def temporary_tables(self):
         """target database supports temporary tables"""

diff --git a/test/sql/test_defaults.py b/test/sql/test_defaults.py
index 7f83917085345887e05513718049d8a45bb20ca8..543ae1f98c51b58a9b95e2be6ddbe27086ada421 100644 (file)
--- a/test/sql/test_defaults.py
+++ b/test/sql/test_defaults.py
@@ -1082,11 +1082,11 @@ class EmptyInsertTest(fixtures.TestBase):
 
 
 class AutoIncrementTest(fixtures.TestBase):
-    __requires__ = ("identity",)
+
     __backend__ = True
 
-    @testing.provide_metadata
-    def test_autoincrement_single_col(self, connection):
+    @testing.requires.empty_inserts
+    def test_autoincrement_single_col(self, metadata, connection):
         single = Table(
             "single", self.metadata, Column("id", Integer, primary_key=True)
         )