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

Load.load_only based on subclass doesn't get recognized #4373

Closed
sqlalchemy-bot opened this issue Nov 23, 2018 · 10 comments
Closed

Load.load_only based on subclass doesn't get recognized #4373

sqlalchemy-bot opened this issue Nov 23, 2018 · 10 comments
Labels
bug Something isn't working loader options ORM options like joinedload(), load_only(), these are complicated and have a lot of issues orm
Milestone

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Dmytro Starosud (@dima-starosud)

This is using SQLAlchemy==1.2.14.
Please consider following code snippet:

from sqlalchemy import Column, Integer, String, create_engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import Load, Query
from sqlalchemy.pool import NullPool

engine = create_engine('postgresql://***', poolclass=NullPool)
connection = engine.connect()
connection.begin_nested()

Base = declarative_base()


class MyEntity(Base):
    __tablename__ = 'my_entity'

    id = Column(Integer, primary_key=True)

    a = Column(String, nullable=False)
    b = Column(String, nullable=False)
    kind = Column(String, nullable=False)

    __mapper_args__ = {'polymorphic_on': kind}


Base.metadata.create_all(connection)


class MyChild(MyEntity):
    __mapper_args__ = {'polymorphic_identity': 'child_1'}


print(MyEntity)
print(Query(MyEntity).options(Load(MyEntity).load_only('a')))
print(MyChild)
print(Query(MyChild).options(Load(MyChild).load_only('a')))

It produces following output:

<class '__main__.MyEntity'>
SELECT my_entity.id AS my_entity_id, my_entity.a AS my_entity_a, my_entity.kind AS my_entity_kind
FROM my_entity
<class '__main__.MyChild'>
SELECT my_entity.id AS my_entity_id, my_entity.a AS my_entity_a, my_entity.b AS my_entity_b, my_entity.kind AS my_entity_kind
FROM my_entity
WHERE my_entity.kind IN (:kind_1)

Please assist. Did I misread documentation? Note unbound load_only('a') works well.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

I think because "a" is part of MyEntity, you'd want to Load(MyEntity).load_only('a'). It probably should work the way you're doing it also, but using Load explicitly is not that common of a use case so I'd stick with unbound.

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • added labels: low priority

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • added labels: orm

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • set milestone to "1.3.xx"

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed title from "Load.load_only on polymorphic child doesn't work" to "Load.load_only based on subclass doesn't get recog"

@sqlalchemy-bot
Copy link
Collaborator Author

Dmytro Starosud (@dima-starosud) wrote:

I think because "a" is part of MyEntity, you'd want to Load(MyEntity).load_only('a').

Looks like it's not really true:

class MyChild(MyEntity):
    __mapper_args__ = {'polymorphic_identity': 'child_1'}
    c = column_property(func.true())

print(Query(MyChild).options(Load(MyChild).load_only('c')))

produces full query

SELECT true() AS true_1, my_entity.id AS my_entity_id, my_entity.a AS my_entity_a, my_entity.b AS my_entity_b, my_entity.kind AS my_entity_kind
FROM my_entity
WHERE my_entity.kind IN (:kind_1)

BTW, It works with aliased:

my_child = aliased(MyChild)
print(Query(my_child).options(Load(my_child).load_only('a')))

produces

SELECT my_entity_1.id AS my_entity_1_id, my_entity_1.a AS my_entity_1_a, my_entity_1.kind AS my_entity_1_kind
FROM my_entity AS my_entity_1
WHERE my_entity_1.kind IN (:kind_1)

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

i meant, Load(MyEntity).load_only("a"). a column_property() on a single-inh subclass is not a use case we have a lot of tests for and that sounds like a separate issue.

@sqlalchemy-bot sqlalchemy-bot added bug Something isn't working orm low priority labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.3.xx milestone Nov 27, 2018
@zzzeek zzzeek added the loader options ORM options like joinedload(), load_only(), these are complicated and have a lot of issues label Nov 27, 2018
@zzzeek
Copy link
Member

zzzeek commented Nov 30, 2018

OK this works for the "unbound" version because of #3287 committed in b63aae2 where an explicit "add the option for superclasses" step was added, when you have query(MyChild).options(load_only('*')). This logic is not used for query(MyChild).options(Load(MyChild).load_only('*')), which is likely because I didn't attend as much to the Load() cases, which may have been either because I overlooked it or that maybe I felt that Load(MyChild) is more specific, but it's not clear if that's true or not. For example, what would we expect if MyChlid has column attributes of its own? Would we expect Load(MyChild).load_only('a') to exclude only columns local to MyChild ? probably not. But this is a behavior change so if I can get it working it will be only for 1.3.

@zzzeek
Copy link
Member

zzzeek commented Nov 30, 2018

OK that was easy

diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index af9bd71b6a..0ef34b0b9b 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -357,9 +357,15 @@ class Load(Generative, MapperOption):
         else:
             effective_path = self.path
 
-        self._set_for_path(
-            self.context, effective_path, replace=True,
-            merge_opts=self.is_opts_only)
+        if effective_path.is_token:
+            for path in effective_path.generate_for_superclasses():
+                self._set_for_path(
+                    self.context, path, replace=True,
+                    merge_opts=self.is_opts_only)
+        else:
+            self._set_for_path(
+                self.context, effective_path, replace=True,
+                merge_opts=self.is_opts_only)
 
     def __getstate__(self):
         d = self.__dict__.copy()

@sqlalchemy-bot
Copy link
Collaborator Author

Mike Bayer has proposed a fix for this issue:

Apply path generation for superclasses to Load._set_path_strategy() https://gerrit.sqlalchemy.org/991

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working loader options ORM options like joinedload(), load_only(), these are complicated and have a lot of issues orm
Projects
None yet
Development

No branches or pull requests

2 participants