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 options aren't looking at the superclass #3287

Closed
sqlalchemy-bot opened this issue Jan 13, 2015 · 15 comments
Closed

Load options aren't looking at the superclass #3287

sqlalchemy-bot opened this issue Jan 13, 2015 · 15 comments
Labels
bug Something isn't working low priority

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by malthe (@malthe)

If a query is made on an alias then the load_only option fails to exclude properties that are set on a base class of the aliased mapper.

q = sess.query(alias).options(load_only("age"))

This is demonstrated in a test case here: https://github.com/malthe/sqlalchemy/tree/load-only-with-alias-issue

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

likely not a bug, you should be using load_only(alias.age). please confirm.

@sqlalchemy-bot
Copy link
Collaborator Author

malthe (@malthe) wrote:

Actually I failed to reproduce the actual problem in this test case.

But there is an issue which is that the alias results in a subselect that lists all the columns in the mapper even though they're not part of the column selection:

select a, b from (select a, b, c from foo join bar on ...)

Note how c is not in the result, but it's still being listed in the subselect.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

it's time to provide a real test case :). Here's a start:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)


e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)

s = Session(e)

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

oh, does your test case show this? I can look there.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

hm, I see a fork of SQLAlchemy there. just give me a quick .py script, thanks.

@sqlalchemy-bot
Copy link
Collaborator Author

malthe (@malthe) wrote:

yeah here's the diff: malthe@dcd43de

I thought I'd write a test case against the code base.

I run the test via:

./sqla_nose.py -sxv -w orm/test_deferred.py

But I can try and isolate to a simple .py file.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

OK, those tests can be helpful when I put the fix in but I need to understand the issue first (I'm fixing something else right now).

@sqlalchemy-bot
Copy link
Collaborator Author

malthe (@malthe) wrote:

Here's a snippet that does demonstrate the issue:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'

    id = Column(Integer, primary_key=True)
    a_name = Column(String)


class B(A):
    __tablename__ = 'b'

    id = Column(Integer, ForeignKey("a.id"), primary_key=True)
    b_name = Column(String)


class C(Base):
    __tablename__ = 'c'

    id = Column(Integer, primary_key=True)
    c_name = Column(String)
    b_id = Column("b_id", Integer, ForeignKey("b.id"))

    b = relationship(B)

e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)

s = Session(e)

b = B(id=1, a_name="foo", b_name="bar")
s.add(b)

c = C(id=1, b_id=b.id, c_name="boo")
s.add(c)

s.flush()

print s.query(C).join(B).options(
    contains_eager(C.b).load_only("id")
)

Note how a.a_name is included, but b.b_name is correctly excluded.

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

ok, so is this the bug?

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'

    id = Column(Integer, primary_key=True)
    a_name = Column(String)

class B(A):
    __tablename__ = 'b'

    id = Column(Integer, ForeignKey("a.id"), primary_key=True)
    b_name = Column(String)


s = Session()

print s.query(B).options(load_only(B.id))

it isn't looking at the superclass. can we rename this issue then? This is all I'm looking at so far.

SELECT b.id AS b_id, a.id AS a_id, a.a_name AS a_a_name 
FROM a JOIN b ON a.id = b.id

@sqlalchemy-bot
Copy link
Collaborator Author

malthe (@malthe) wrote:

I think that's the bug. Let me rename the issue.

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by malthe (@malthe):

  • changed title from "Load options ignore inherited classes with alias" to "Load options aren't looking at the superclass"

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

  • The "wildcard" loader options, in particular the one set up by
    the :func:.orm.load_only option to cover all attributes not
    explicitly mentioned, now takes into account the superclasses
    of a given entity, if that entity is mapped with inheritance mapping,
    so that attribute names within the superclasses are also omitted
    from the load. Additionally, the polymorphic discriminator column
    is unconditionally included in the list, just in the same way that
    primary key columns are, so that even with load_only() set up,
    polymorphic loading of subtypes continues to function correctly.
    fixes Load options aren't looking at the superclass #3287

b63aae2

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

  • The "wildcard" loader options, in particular the one set up by
    the :func:.orm.load_only option to cover all attributes not
    explicitly mentioned, now takes into account the superclasses
    of a given entity, if that entity is mapped with inheritance mapping,
    so that attribute names within the superclasses are also omitted
    from the load. Additionally, the polymorphic discriminator column
    is unconditionally included in the list, just in the same way that
    primary key columns are, so that even with load_only() set up,
    polymorphic loading of subtypes continues to function correctly.
    fixes Load options aren't looking at the superclass #3287

(cherry picked from commit b63aae2)

Conflicts:
lib/sqlalchemy/orm/mapper.py

11383da

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot
Copy link
Collaborator Author

malthe (@malthe) wrote:

This solves the problem.

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

No branches or pull requests

1 participant