query evalutator needs to evaluate *before* the statement

Issue #2122 resolved
Mike Bayer repo owner created an issue
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.

Comments (3)

  1. Log in to comment