Skip to content

query evalutator needs to evaluate *before* the statement #2122

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

Closed
sqlalchemy-bot opened this issue Apr 7, 2011 · 3 comments
Closed

query evalutator needs to evaluate *before* the statement #2122

sqlalchemy-bot opened this issue Apr 7, 2011 · 3 comments
Labels
bug Something isn't working high priority orm
Milestone

Comments

@sqlalchemy-bot
Copy link
Collaborator

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

from sqlalchemy import create_engine, MetaData, Column, Unicode
from sqlalchemy.orm import sessionmaker
from sqlalchemy.ext.declarative import declarative_base

engine = create_engine('sqlite:///:memory:')
metadata = MetaData(bind = engine)
session = sessionmaker(bind = engine)()
Base = declarative_base(metadata = metadata)

class Entity(Base):
   __tablename__ = 'entity'
   name = Column(Unicode(128), primary_key = True)

metadata.create_all()
e = Entity(name = u'hello')
session.add(e)
session.flush()
session.expire(e)
session.query(Entity).filter_by(name = u'hello').delete()




#!diff
diff -r ed5783772dff7bdef9de381dd0fb5c70adc89995 lib/sqlalchemy/orm/query.py
--- a/lib/sqlalchemy/orm/query.py	Wed Apr 06 10:36:33 2011 -0400
+++ b/lib/sqlalchemy/orm/query.py	Thu Apr 07 11:43:07 2011 -0400
@@ -2138,6 +2138,9 @@
 
         session = self.session
 
+        if self._autoflush:
+            session._autoflush()
+
         if synchronize_session == 'evaluate':
             try:
                 evaluator_compiler = evaluator.EvaluatorCompiler()
@@ -2154,9 +2157,16 @@
                     "Specify 'fetch' or False for the synchronize_session "
                     "parameter.")
 
-        delete_stmt = sql.delete(primary_table, context.whereclause)
-
-        if synchronize_session == 'fetch':
+            target_cls = self._mapper_zero().class_
+
+            #TODO: detect when the where clause is a trivial primary key match
+            objs_to_expunge = [                               obj for (cls, pk),obj in
+                                session.identity_map.iteritems()
+                                if issubclass(cls, target_cls) and
+                                eval_condition(obj)](
+)
+
+        elif synchronize_session == 'fetch':
             #TODO: use RETURNING when available
             select_stmt = context.statement.with_only_columns(
                                                 primary_table.primary_key)
@@ -2164,19 +2174,11 @@
                                         select_stmt,
                                         params=self._params).fetchall()
 
-        if self._autoflush:
-            session._autoflush()
+        delete_stmt = sql.delete(primary_table, context.whereclause)
+
         result = session.execute(delete_stmt, params=self._params)
 
         if synchronize_session == 'evaluate':
-            target_cls = self._mapper_zero().class_
-
-            #TODO: detect when the where clause is a trivial primary key match
-            objs_to_expunge = [                               obj for (cls, pk),obj in
-                                session.identity_map.iteritems()
-                                if issubclass(cls, target_cls) and
-                                eval_condition(obj)](
-)
             for obj in objs_to_expunge:
                 session._remove_newly_deleted(attributes.instance_state(obj))
         elif synchronize_session == 'fetch':
@@ -2271,6 +2273,9 @@
 
         session = self.session
 
+        if self._autoflush:
+            session._autoflush()
+
         if synchronize_session == 'evaluate':
             try:
                 evaluator_compiler = evaluator.EvaluatorCompiler()
@@ -2291,43 +2296,45 @@
                         "Could not evaluate current criteria in Python. "
                         "Specify 'fetch' or False for the "
                         "synchronize_session parameter.")
-
-        update_stmt = sql.update(primary_table, context.whereclause, values)
-
-        if synchronize_session == 'fetch':
+            target_cls = self._mapper_zero().class_
+            matched_objects = [           for (cls, pk),obj in session.identity_map.iteritems():
+                evaluated_keys = value_evaluators.keys()
+
+                if issubclass(cls, target_cls) and eval_condition(obj):
+                    matched_objects.append(obj)
+
+        elif synchronize_session == 'fetch':
             select_stmt = context.statement.with_only_columns(
                                                 primary_table.primary_key)
             matched_rows = session.execute(
                                         select_stmt,
                                         params=self._params).fetchall()
 
-        if self._autoflush:
-            session._autoflush()
+        update_stmt = sql.update(primary_table, context.whereclause, values)
+
         result = session.execute(update_stmt, params=self._params)
 
         if synchronize_session == 'evaluate':
             target_cls = self._mapper_zero().class_
 
-            for (cls, pk),obj in session.identity_map.iteritems():
-                evaluated_keys = value_evaluators.keys()
-
-                if issubclass(cls, target_cls) and eval_condition(obj):
-                    state, dict_ = attributes.instance_state(obj),\
-                                            attributes.instance_dict(obj)
-
-                    # only evaluate unmodified attributes
-                    to_evaluate = state.unmodified.intersection(
-                                                            evaluated_keys)
-                    for key in to_evaluate:
-                        dict_[key](]
+) = value_evaluators[key](key)(obj)
-
-                    state.commit(dict_, list(to_evaluate))
-
-                    # expire attributes with pending changes 
-                    # (there was no autoflush, so they are overwritten)
-                    state.expire_attributes(dict_,
-                                    set(evaluated_keys).
-                                        difference(to_evaluate))
+            for obj in matched_objects:
+                state, dict_ = attributes.instance_state(obj),\
+                                        attributes.instance_dict(obj)
+
+                # only evaluate unmodified attributes
+                to_evaluate = state.unmodified.intersection(
+                                                        evaluated_keys)
+                for key in to_evaluate:
+                    dict_[key](key) = value_evaluators[key](key)(obj)
+
+                state.commit(dict_, list(to_evaluate))
+
+                # expire attributes with pending changes 
+                # (there was no autoflush, so they are overwritten)
+                state.expire_attributes(dict_,
+                                set(evaluated_keys).
+                                    difference(to_evaluate))
 
         elif synchronize_session == 'fetch':
             target_mapper = self._mapper_zero()

tests:

  1. autoflush occurs before "fetch" proceeds. need an object with pending changes that make it part of the update or delete
  2. evaluation occurs after autoflush, but before the statement emitted. Need to ensure fetch and evaluate catch objects when the update statement modifies the keys in the where clause; same for deleted. also confirm that old behavior didn't work in these cases.
@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • added labels: high priority

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

708a25e

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

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

No branches or pull requests

1 participant