Skip to content

eager_defaults shouldn't be needed for pk returning even if cols are not actual pks #9082

Description

@zzzeek

Discussed in #9080

simple case can be reproduced on sqlite w/ new returning support

from sqlalchemy import Column
from sqlalchemy import create_engine
from sqlalchemy import DateTime
from sqlalchemy import func
from sqlalchemy import String
from sqlalchemy.orm import declarative_base
from sqlalchemy.orm import Session
Base = declarative_base()


class A(Base):
    __tablename__ = 'a'

    date = Column(
        DateTime,
        server_default=func.now(),

        # uncomment to succeed
        # primary_key=True
    )
    data = Column(String)

    __mapper_args__ = {"primary_key": [date], "eager_defaults": False}


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

s = Session(e)

s.add(A(data='asdf'))
s.commit()

fails because it didnt put the col in return_defaults(), which it should, or raise an error saying eager_defaults has to be turned on, or something

 File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/unitofwork.py", line 488, in finalize_flush_changes
    self.session._register_persistent(other)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/session.py", line 3162, in _register_persistent
    instance_key = mapper._identity_key_from_state(state)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/mapper.py", line 3363, in _identity_key_from_state
    [
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/mapper.py", line 3364, in <listcomp>
    manager[prop.key].impl.get(state, dict_, passive)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/attributes.py", line 1075, in get
    value = self._fire_loader_callables(state, key, passive)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/attributes.py", line 1105, in _fire_loader_callables
    return state._load_expired(state, passive)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/state.py", line 807, in _load_expired
    self.manager.expired_attribute_loader(self, toload, passive)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/orm/loading.py", line 1573, in load_scalar_attributes
    raise sa_exc.InvalidRequestError(
sqlalchemy.exc.InvalidRequestError: Instance <A at 0x7f90c82e18a0> cannot be refreshed - it's not  persistent and does not contain a full primary key.

turning on eager_defaults, or just leaving it at the new "auto" setting, works, but this is still spooky and we should document this case and/or support it directly.

2.0 final is tentative, will likely move to 2.x

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions