"implicit combining column" warning not emitting for two columns from two different aliases against same parent table

[moriyoshi]

trafficstars

Describe the bug

Description

The following code yields a strange result. I'm expecting bar_1, bar_2, and bar_3 to be mapped as CompositeBar(value_1="a", value_2="b"), CompositeBar(value_1="c", value_2="d"), and CompositeBar(value_1="e", value_2="f") respectively, but actually it ended up with every CompositeBar being CompositeBar(value_1="e", value_2="f") as if it got mapped to bar_alias_3 and the rest were ignored.

BTW the reason why I didn't use relationships here is that the actual code I'm working on has more complex mappings like the composite instance gets mapped to both the table that corresponds to the containing class, and the table that it refers to.

Optional link from https://docs.sqlalchemy.org which documents the behavior that is expected

No response

SQLAlchemy Version in Use

2.0.25

DBAPI (i.e. the database driver)

sqlite3 module

Database Vendor and Major Version

SQLite 2.6.0

Python Version

3.8.16

Operating system

macOS

To Reproduce

import dataclasses

import sqlalchemy as sa
from sqlalchemy import orm


class Base(orm.MappedAsDataclass, orm.DeclarativeBase):
    pass


class Bar(Base):
    __tablename__ = "bar"

    id: orm.Mapped[int] = sa.Column(sa.Integer, primary_key=True)
    value_1: orm.Mapped[str] = sa.Column(sa.String, nullable=False)
    value_2: orm.Mapped[str] = sa.Column(sa.String, nullable=False)


@dataclasses.dataclass
class CompositeBar:
    value_1: str
    value_2: str


foo_table = sa.Table(
    "foo",
    Base.metadata,
    sa.Column("id", sa.Integer, primary_key=True),
    sa.Column("bar_1_id", sa.Integer, sa.ForeignKey(Bar.id), nullable=False),
    sa.Column("bar_2_id", sa.Integer, sa.ForeignKey(Bar.id), nullable=False),
    sa.Column("bar_3_id", sa.Integer, sa.ForeignKey(Bar.id), nullable=False),
)
bar_alias_1 = orm.aliased(Bar)
bar_alias_2 = orm.aliased(Bar)
bar_alias_3 = orm.aliased(Bar)


class Foo(Base):
    __table__ = foo_table.outerjoin(
        bar_alias_1,
        foo_table.c.bar_1_id == bar_alias_1.id
    ).outerjoin(
        bar_alias_2,
        foo_table.c.bar_2_id == bar_alias_2.id
    ).outerjoin(
        bar_alias_3,
        foo_table.c.bar_3_id == bar_alias_3.id
    )

    id: orm.Mapped[int] = orm.column_property(foo_table.c.id)
    _bar_1_id: orm.Mapped[int] = orm.column_property(foo_table.c.bar_1_id, bar_alias_1.id)
    _bar_2_id: orm.Mapped[int] = orm.column_property(foo_table.c.bar_2_id, bar_alias_2.id)
    _bar_3_id: orm.Mapped[int] = orm.column_property(foo_table.c.bar_3_id, bar_alias_3.id)
    bar_1: orm.Mapped[CompositeBar] = orm.composite(CompositeBar, bar_alias_1.value_1, bar_alias_1.value_2)
    bar_2: orm.Mapped[CompositeBar] = orm.composite(CompositeBar, bar_alias_2.value_1, bar_alias_2.value_2)
    bar_3: orm.Mapped[CompositeBar] = orm.composite(CompositeBar, bar_alias_3.value_1, bar_alias_3.value_2)


engine = sa.create_engine("sqlite:///")
Base.metadata.create_all(bind=engine)

with orm.Session(bind=engine) as session:
    bar_1 = Bar(id=1, value_1="a", value_2="b")
    bar_2 = Bar(id=2, value_1="c", value_2="d")
    bar_3 = Bar(id=3, value_1="e", value_2="f")
    session.add_all([bar_1, bar_2, bar_3])
    session.flush()
    session.execute(sa.insert(foo_table).values([{"id": 1, "bar_1_id": bar_1.id, "bar_2_id": bar_2.id, "bar_3_id": bar_3.id}]))
    foo = session.execute(sa.select(Foo).where(Foo.id == 1)).scalar()
    print(foo)

Error

Actual result:

Foo(id=1, _bar_1_id=1, _bar_2_id=2, _bar_3_id=3, bar_1=CompositeBar(value_1='e', value_2='f'), bar_2=CompositeBar(value_1='e', value_2='f'), bar_3=CompositeBar(value_1='e', value_2='f'))

Expected:

Foo(id=1, _bar_1_id=1, _bar_2_id=2, _bar_3_id=3, bar_1=CompositeBar(value_1='a', value_2='b'), bar_2=CompositeBar(value_1='c', value_2='d'), bar_3=CompositeBar(value_1='e', value_2='f'))

Additional context

No response

[zzzeek]

hi

Composite maps the columns individually, then creates an accessor that generates the composite object from those mapped attributes on your class. Here you have six attributes with three duplicate names each so they are being lumped together and/or overwriting each other.

Map the columns with distinct names then create the composites against those names:

class Foo(Base):
    __table__ = (
        foo_table.outerjoin(
            bar_alias_1, foo_table.c.bar_1_id == bar_alias_1.id
        )
        .outerjoin(bar_alias_2, foo_table.c.bar_2_id == bar_alias_2.id)
        .outerjoin(bar_alias_3, foo_table.c.bar_3_id == bar_alias_3.id)
    )

    id: orm.Mapped[int] = orm.column_property(foo_table.c.id)
    _bar_1_id: orm.Mapped[int] = orm.column_property(
        foo_table.c.bar_1_id, bar_alias_1.id
    )
    _bar_2_id: orm.Mapped[int] = orm.column_property(
        foo_table.c.bar_2_id, bar_alias_2.id
    )
    _bar_3_id: orm.Mapped[int] = orm.column_property(
        foo_table.c.bar_3_id, bar_alias_3.id
    )
    b1v1 = orm.column_property(bar_alias_1.value_1)
    b1v2 = orm.column_property(bar_alias_1.value_2)
    b2v1 = orm.column_property(bar_alias_2.value_1)
    b2v2 = orm.column_property(bar_alias_2.value_2)
    b3v1 = orm.column_property(bar_alias_3.value_1)
    b3v2 = orm.column_property(bar_alias_3.value_2)

    bar_1: orm.Mapped[CompositeBar] = orm.composite(
        CompositeBar, "b1v1", "b1v2" #bar_alias_1.value_1, bar_alias_1.value_2
    )
    bar_2: orm.Mapped[CompositeBar] = orm.composite(
        CompositeBar, "b2v1", "b2v2", #bar_alias_2.value_1, bar_alias_2.value_2
    )
    bar_3: orm.Mapped[CompositeBar] = orm.composite(
        CompositeBar, "b3v1", "b3v2", #bar_alias_3.value_1, bar_alias_3.value_2
    )

I don't have a fix for this in 2.0 as it would require me to entirely rearchitect how Composite gets these values.

[zzzeek]

Hmm OK actually I think my approach above is how this should be mapped. what's failing is that the warning that tells you it's implicitly combining two columns is not happening for this very specific situation, which is the warning that would let you know you need to map these columns individually.

[moriyoshi]

Superthanks for the clarification! I was looking into the root cause and I just gave up as it would require somewhat thorough overhaul to get this right. Looking forward to the fix.