Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
moriyoshi opened this issue Feb 3, 2024 · 3 comments
Labels
bug Something isn't working orm
Milestone

Comments

@moriyoshi
Copy link
Sponsor

moriyoshi commented Feb 3, 2024

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

@moriyoshi moriyoshi added the requires triage New issue that requires categorization label Feb 3, 2024
@moriyoshi moriyoshi changed the title Composite columns with joined FromClause yields strange result. Composite properties with joined FromClause yields a strange result Feb 3, 2024
@zzzeek
Copy link
Member

zzzeek commented Feb 3, 2024

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 zzzeek changed the title Composite properties with joined FromClause yields a strange result Composite relies on mapped attribute names which doesnt work if those attributes were implicitly named from multiple, same-named colums Feb 3, 2024
@zzzeek zzzeek added bug Something isn't working orm quagmire really hard to make the issue work "correctly" without lots of complication, risk hard orm hard ORM bugs mike needs to look at and removed requires triage New issue that requires categorization labels Feb 3, 2024
@zzzeek zzzeek added this to the 2.1 milestone Feb 3, 2024
@zzzeek zzzeek changed the title Composite relies on mapped attribute names which doesnt work if those attributes were implicitly named from multiple, same-named colums Composite relies on mapped attribute names which doesnt work if those attributes were implicitly named from multiple, same-named columns Feb 3, 2024
@zzzeek
Copy link
Member

zzzeek commented Feb 4, 2024

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.

@zzzeek zzzeek changed the title Composite relies on mapped attribute names which doesnt work if those attributes were implicitly named from multiple, same-named columns "implicit combining column" warning not emitting for two columns from two different aliases against same parent table Feb 4, 2024
@zzzeek zzzeek removed quagmire really hard to make the issue work "correctly" without lots of complication, risk hard orm hard ORM bugs mike needs to look at labels Feb 4, 2024
@zzzeek zzzeek modified the milestones: 2.1, 2.0.x Feb 4, 2024
@moriyoshi
Copy link
Sponsor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working orm
Projects
None yet
Development

No branches or pull requests

2 participants