diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2023-02-18 09:10:20 -0500 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2023-02-18 09:11:36 -0500 |
commit | ebcb80be9ce27d9cd383dc7c0750569c43587df2 (patch) | |
tree | 4e8ab4d2c0df8a2cedf6a20b17070beb29e10dff | |
parent | c6ecc39b62b41c4f21375a51bac582c61baa3d9d (diff) | |
download | sqlalchemy-ebcb80be9ce27d9cd383dc7c0750569c43587df2.tar.gz |
consider column.name directly when evaluating use_existing_column
Fixed issue where new :paramref:`_orm.mapped_column.use_existing_column`
feature would not work if the two same-named columns were mapped under
attribute names that were differently-named from the explicit name given to
the column itself. The attribute names can now be differently named when
using this parameter.
Fixes: #9332
Change-Id: I43716b8ca2b089e54a2b078db28b6c4770468bdd
-rw-r--r-- | doc/build/changelog/unreleased_20/9332.rst | 9 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/properties.py | 3 | ||||
-rw-r--r-- | test/orm/declarative/test_inheritance.py | 151 |
3 files changed, 116 insertions, 47 deletions
diff --git a/doc/build/changelog/unreleased_20/9332.rst b/doc/build/changelog/unreleased_20/9332.rst new file mode 100644 index 000000000..ce3cc9161 --- /dev/null +++ b/doc/build/changelog/unreleased_20/9332.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm declarative + :tickets: 9332 + + Fixed issue where new :paramref:`_orm.mapped_column.use_existing_column` + feature would not work if the two same-named columns were mapped under + attribute names that were differently-named from an explicit name given to + the column itself. The attribute names can now be differently named when + using this parameter. diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index e736e4fd2..a5f34f3de 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -663,8 +663,9 @@ class MappedColumn( ) supercls_mapper = class_mapper(decl_scan.inherits, False) + colname = column.name if column.name is not None else key column = self.column = supercls_mapper.local_table.c.get( # type: ignore # noqa: E501 - key, column + colname, column ) if column.key is None: diff --git a/test/orm/declarative/test_inheritance.py b/test/orm/declarative/test_inheritance.py index 02d285297..4cc086be2 100644 --- a/test/orm/declarative/test_inheritance.py +++ b/test/orm/declarative/test_inheritance.py @@ -43,7 +43,11 @@ class DeclarativeTestBase(fixtures.TestBase, testing.AssertsExecutionResults): Base.metadata.drop_all(testing.db) -class DeclarativeInheritanceTest(DeclarativeTestBase): +class DeclarativeInheritanceTest( + testing.AssertsCompiledSQL, DeclarativeTestBase +): + __dialect__ = "default" + @testing.emits_warning(r".*does not indicate a 'polymorphic_identity'") def test_we_must_copy_mapper_args(self): class Person(Base): @@ -810,13 +814,18 @@ class DeclarativeInheritanceTest(DeclarativeTestBase): ) @testing.variation("decl_type", ["legacy", "use_existing_column"]) + @testing.variation("different_attr", [True, False]) def test_columns_single_inheritance_conflict_resolution( - self, connection, decl_base, decl_type + self, connection, decl_base, decl_type, different_attr ): """Test that a declared_attr can return the existing column and it will be ignored. this allows conditional columns to be added. - See [ticket:2472]. + See #2472. + + use_existing_column variant is #8822 + + different_attr variant is #9332 """ @@ -825,58 +834,108 @@ class DeclarativeInheritanceTest(DeclarativeTestBase): id = Column(Integer, Identity(), primary_key=True) class Engineer(Person): - - """single table inheritance""" - - if decl_type.legacy: - - @declared_attr - def target_id(cls): - return cls.__table__.c.get( - "target_id", Column(Integer, ForeignKey("other.id")) + if different_attr: + if decl_type.legacy: + + @declared_attr + def e_target_id(cls): + return cls.__table__.c.get( + "target_id", + Column( + "target_id", Integer, ForeignKey("other.id") + ), + ) + + elif decl_type.use_existing_column: + e_target_id: Mapped[int] = mapped_column( + "target_id", + ForeignKey("other.id"), + use_existing_column=True, + ) + else: + if decl_type.legacy: + + @declared_attr + def target_id(cls): + return cls.__table__.c.get( + "target_id", + Column(Integer, ForeignKey("other.id")), + ) + + elif decl_type.use_existing_column: + target_id: Mapped[int] = mapped_column( + ForeignKey("other.id"), use_existing_column=True ) - elif decl_type.use_existing_column: - target_id: Mapped[int] = mapped_column( - ForeignKey("other.id"), use_existing_column=True - ) - - @declared_attr - def target(cls): - return relationship("Other") + target = relationship("Other") class Manager(Person): - - """single table inheritance""" - - if decl_type.legacy: - - @declared_attr - def target_id(cls): - return cls.__table__.c.get( - "target_id", Column(Integer, ForeignKey("other.id")) + if different_attr: + if decl_type.legacy: + + @declared_attr + def m_target_id(cls): + return cls.__table__.c.get( + "target_id", + Column( + "target_id", Integer, ForeignKey("other.id") + ), + ) + + elif decl_type.use_existing_column: + m_target_id: Mapped[int] = mapped_column( + "target_id", + ForeignKey("other.id"), + use_existing_column=True, + ) + else: + if decl_type.legacy: + + @declared_attr + def target_id(cls): + return cls.__table__.c.get( + "target_id", + Column(Integer, ForeignKey("other.id")), + ) + + elif decl_type.use_existing_column: + target_id: Mapped[int] = mapped_column( + ForeignKey("other.id"), use_existing_column=True ) - elif decl_type.use_existing_column: - target_id: Mapped[int] = mapped_column( - ForeignKey("other.id"), use_existing_column=True - ) - - @declared_attr - def target(cls): - return relationship("Other") + target = relationship("Other") class Other(decl_base): __tablename__ = "other" id = Column(Integer, Identity(), primary_key=True) - is_( - Engineer.target_id.property.columns[0], - Person.__table__.c.target_id, - ) - is_( - Manager.target_id.property.columns[0], Person.__table__.c.target_id - ) + if different_attr: + is_( + Engineer.e_target_id.property.columns[0], + Person.__table__.c.target_id, + ) + is_( + Manager.m_target_id.property.columns[0], + Person.__table__.c.target_id, + ) + self.assert_compile( + select(Engineer.e_target_id), + "SELECT person.target_id FROM person", + ) + self.assert_compile( + select(Manager.m_target_id), + "SELECT person.target_id FROM person", + ) + else: + is_( + Engineer.target_id.property.columns[0], + Person.__table__.c.target_id, + ) + is_( + Manager.target_id.property.columns[0], + Person.__table__.c.target_id, + ) + # do a brief round trip on this decl_base.metadata.create_all(connection) with Session(connection) as session: @@ -946,11 +1005,11 @@ class DeclarativeInheritanceTest(DeclarativeTestBase): @testing.variation("decl_type", ["legacy", "use_existing_column"]) def test_columns_single_inheritance_cascading_resolution_pk( - self, decl_type + self, decl_type, decl_base ): """An additional test for #4352 in terms of the requested use case.""" - class TestBase(Base): + class TestBase(decl_base): __abstract__ = True if decl_type.legacy: |