Skip to content

major inefficiency in eager_defaults #3909

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.

Sign up for GitHub

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

Jump to bottom
Closed
sqlalchemy-bot opened this issue Feb 9, 2017 · 4 comments
Closed

major inefficiency in eager_defaults #3909

sqlalchemy-bot opened this issue Feb 9, 2017 · 4 comments
Labels
bug Something isn't working orm
Milestone
1.1

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Michael Bayer (@zzzeek)

When "return_defaults" is used, we put all columns that are unspecified into RETURNING. However, we dont put columns that we've specified NULL for, as these are explicit in the INSERT. Then we do the SELECT, totally ruining the point of doing the RETURNING. eager_defaults should ensure that only columns that have defaults are actually in the list of columns we care about. We should not populate columns that we are explicitly passing NULL.

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)
    x = Column(Integer)

    __mapper_args__ = {
        "eager_defaults": True
    }

e = create_engine("postgresql://scott:tiger@localhost/test", echo=True)
Base.metadata.create_all(e)

s = Session(e)

s.add(A())
s.flush()

output:

2017-02-08 21:15:35,375 INFO sqlalchemy.engine.base.Engine INSERT INTO a (x) VALUES (%(x)s) RETURNING a.id
2017-02-08 21:15:35,375 INFO sqlalchemy.engine.base.Engine {'x': None}
2017-02-08 21:15:35,378 INFO sqlalchemy.engine.base.Engine SELECT a.x AS a_x 
FROM a 
WHERE a.id = %(param_1)s
2017-02-08 21:15:35,378 INFO sqlalchemy.engine.base.Engine {'param_1': 1}

This should likely be for 1.2.
@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

if no existing tests break then this should be good for 1.1, eager_defaults is about "defaults", not columns without any default

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed milestone from "1.2" to "1.1"

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

Don't post-SELECT columns w/o a server default/onupdate for eager_defaults

Fixed a major inefficiency in the "eager_defaults" feature whereby
an unnecessary SELECT would be emitted for column values where the
ORM had explicitly inserted NULL, corresponding to attributes that
were unset on the object but did not have any server default
specified, as well as expired attributes on update that nevertheless
had no server onupdate set up. As these columns are not part of the
RETURNING that eager_defaults tries to use, they should not be
post-SELECTed either.

Change-Id: I0d4f1e9d3d9717d68dcc0592f69456a1f1c36df8
Fixes: #3909

→ 000e960

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot sqlalchemy-bot added bug Something isn't working orm labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.1 milestone Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working orm
Projects
None yet
Milestone
1.1
Development

No branches or pull requests
1 participant
@sqlalchemy-bot