fix in issue 10365 also fixed an ORM select() issue that likely has no test coverage about sqlalchemy

[zzzeek]

hi -

this is a very clear, straightforward bug report with a very clear assertion which then...does not reproduce for me at all. the query at the end has the extra criteria and we have a lot of tests for this:

SELECT address.id 
FROM user_account JOIN address ON address.id = user_account.address_id 
WHERE user_account.model_type IN (__[POSTCOMPILE_model_type_1])

full test

from sqlalchemy import select
from sqlalchemy import ForeignKey, String
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column


class Base(DeclarativeBase):
     pass

class User(Base):
     __tablename__ = "user_account"

     id: Mapped[int] = mapped_column(primary_key=True)
     address_id: Mapped[int] = mapped_column(ForeignKey("address.id"))
     model_type: Mapped[str] = mapped_column(String(50))
     __mapper_args__ = {"polymorphic_identity": "base", "polymorphic_on": "model_type"}


class Admin(User):
     __mapper_args__ = {"polymorphic_identity": "admin"}

class Address(Base):
     __tablename__ = "address"
     id: Mapped[int] = mapped_column(primary_key=True)

print(select(Address.id).select_from(Admin).join(Address))

[zzzeek]

here's sqlite

 SELECT address.id 
FROM user_account JOIN address ON address.id = user_account.address_id 
WHERE user_account.model_type IN (?)
2024-05-24 10:39:33,361 INFO sqlalchemy.engine.Engine [generated in 0.00067s] ('admin',)

postgresql

SELECT address.id 
FROM user_account JOIN address ON address.id = user_account.address_id 
WHERE user_account.model_type IN (%(model_type_1_1)s)
2024-05-24 10:40:07,564 INFO sqlalchemy.engine.Engine [generated in 0.00009s] {'model_type_1_1': 'admin'}

do you have any event handlers modifying queries ? or is that not an accurate version of your real world test?

[jdimmerman]

Odd! I reproduced locally and then again in a python fiddle with my minimalist example: https://python-fiddle.com/saved/Mf8Y0oUm37DfVyOINqjQ

I'm still able to reproduce with your example

[zzzeek]

has your SQLAlchemy installation been modified? can you verify you're on 2.0.30? it looks like older 2.0.x versions do have this bug, so likely an upgrade needed

[jdimmerman]

Confirming my local version now. Indeed the python fiddle is using 2.0.20

[zzzeek]

it looks like this behavior changed in 2.0.22 with #10365 . so there's something concerning which is that that issue wasn't targeting this behavior so we may need to add a more direct test

[jdimmerman]

I'm still seeing the issue locally on 2.0.30 but my local configuration is much more complex, which may or may not be a contributing factor. I'll try and match your reproduction and go from there. Thanks for the quick feedback.

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the main branch:

Add test for issue 11412 https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5305

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the rel_2_0 branch:

Add test for issue 11412 https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5306

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the rel_1_4 branch:

Add test for issue 11412 https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5307

[jdimmerman]

Ah so I can still replicate the issue with the legacy ORM syntax, but agreed the query syntax fix is good on 2.0.30.

Issue exists in:

print(session.query(Admin).join(Address).with_entities(Address.id))

[jdimmerman]

Full reproduction on 2.0.30

from sqlalchemy import select
from sqlalchemy import ForeignKey, String
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column, Query


class Base(DeclarativeBase):
     pass

class User(Base):
     __tablename__ = "user_account"

     id: Mapped[int] = mapped_column(primary_key=True)
     address_id: Mapped[int] = mapped_column(ForeignKey("address.id"))
     model_type: Mapped[str] = mapped_column(String(50))
     __mapper_args__ = {"polymorphic_identity": "base", "polymorphic_on": "model_type"}


class Admin(User):
     __mapper_args__ = {"polymorphic_identity": "admin"}

class Address(Base):
     __tablename__ = "address"
     id: Mapped[int] = mapped_column(primary_key=True)

# this includes the WHERE clause (good, not a bug)
print(select(Address.id).select_from(Admin).join(Address))

# this is missing the WHERE clause (incorrect, this is a bug)
print(Query(Admin).join(Address).with_entities(Address.id))

[zzzeek]

that's expected, with_entities() replaces what you passed to Query() as though it never happened.

use select_from() here as well

print(Query(Admin).select_from(Admin).join(Address).with_entities(Address.id))

[jdimmerman]

OK I see. I find it odd that it still retains the reference to the underlying table user_account but in either case, easy for us to be aware of (and we should migrate to the newer syntax anyways).
Thanks for the time/help!

[zzzeek]

Query converts to a select() internally but only at the end. it's all legacy stuff that we dont really want to mess with if we dont have to.

[jdimmerman]

Understood!