sqlalchemy / sqlalchemy

The Database Toolkit for Python
https://www.sqlalchemy.org
MIT License
9.62k stars 1.43k forks source link

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

Closed jdimmerman closed 5 months ago

jdimmerman commented 5 months ago

Describe the bug

A query including a polymorphic child but not selecting from polymorphic column set excludes the polymorphic filter. This impacts with select() and with_entities().

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

No response

SQLAlchemy Version in Use

2.0.30

DBAPI (i.e. the database driver)

psycopg2, though I replicated without specifying a driver in pyfiddle

Database Vendor and Major Version

PostgreSQL, though appears agnostic to vendor

Python Version

3.12.2

Operating system

Mac, Linux

To Reproduce

Consider the configuration:

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, Admin.id).select_from(Admin).join(Address))

Results in the following. as expect. Note the WHERE clause.

SELECT address.id, user_account.id AS id_1 
FROM user_account JOIN address ON address.id = user_account.address_id 
WHERE user_account.model_type IN ('admin')
print(select(Address.id).select_from(Admin).join(Address))

Results in the following. Note the absence of the WHERE clause.

SELECT address.id
FROM user_account JOIN address ON address.id = user_account.address_id 


### Error

Note the missing WHERE clause in the reproduction

### Additional context

_No response_
zzzeek commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

image

zzzeek commented 5 months ago

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 commented 5 months ago

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

zzzeek commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

Understood!