Commits

Mike Bayer  committed e5b698b

- session.refresh() raises an informative error message if
the list of attributes does not include any column-based
attributes.

- query() raises an informative error message if no columns
or mappers are specified.

- lazy loaders now trigger autoflush before proceeding. This
allows expire() of a collection or scalar relation to
function properly in the context of autoflush.

- whitespace fix to new Table prefixes option

  • Participants
  • Parent commits afb7530

Comments (0)

Files changed (8)

     - In addition to expired attributes, deferred attributes
       also load if their data is present in the result set.
       [ticket:870]
+    
+    - session.refresh() raises an informative error message if
+      the list of attributes does not include any column-based
+      attributes.
+    
+    - query() raises an informative error message if no columns
+      or mappers are specified.
+      
+    - lazy loaders now trigger autoflush before proceeding.  This
+      allows expire() of a collection or scalar relation to 
+      function properly in the context of autoflush.
       
     - column_property() attributes which represent SQL expressions
       or columns that are not present in the mapped tables

File lib/sqlalchemy/orm/properties.py

     def cascade_iterator(self, type_, state, visited_instances, halt_on=None):
         if not type_ in self.cascade:
             return
-        passive = type_ != 'delete' or self.passive_deletes
+        # only actively lazy load on the 'delete' cascade
+        passive = type_ != 'delete' or self.passive_deletes  
         mapper = self.mapper.primary_mapper()
         instances = state.value_as_iterable(self.key, passive=passive)
         if instances:

File lib/sqlalchemy/orm/query.py

     def _execute_and_instances(self, querycontext):
         result = self.session.execute(querycontext.statement, params=self._params, mapper=self._mapper_zero_or_none(), _state=self._refresh_state)
         return self.instances(result, querycontext)
-
+    
     def instances(self, cursor, __context=None):
         """Given a ResultProxy cursor as returned by connection.execute(), return an ORM result as an iterator.
 
             labels = dict([(label, property(util.itemgetter(i))) for i, label in enumerate(labels) if label])
             rowtuple = type.__new__(type, "RowTuple", (tuple,), labels)
             rowtuple.keys = labels.keys
-            
+        
         while True:
             context.progress = util.Set()
             context.partials = {}
 
         if lockmode is not None:
             q._lockmode = lockmode
-        q.__get_options(populate_existing=bool(refresh_state), version_check=(lockmode is not None), only_load_props=only_load_props, refresh_state=refresh_state)
+        q.__get_options(
+            populate_existing=bool(refresh_state), 
+            version_check=(lockmode is not None), 
+            only_load_props=only_load_props, 
+            refresh_state=refresh_state)
         q._order_by = None
         try:
             # call using all() to avoid LIMIT compilation complexity
             return None
 
     def _select_args(self):
-        return {'limit':self._limit, 'offset':self._offset, 'distinct':self._distinct, 'group_by':self._group_by or None, 'having':self._having or None}
+        return {
+            'limit':self._limit, 
+            'offset':self._offset, 
+            'distinct':self._distinct, 
+            'group_by':self._group_by or None, 
+            'having':self._having or None
+        }
     _select_args = property(_select_args)
 
     def _should_nest_selectable(self):
         
         self._adjust_for_single_inheritance(context)
         
+        if not context.primary_columns:
+            if self._only_load_props:
+                raise sa_exc.InvalidRequestError("No column-based properties specified for refresh operation."
+                " Use session.expire() to reload collections and related items.")
+            else:
+                raise sa_exc.InvalidRequestError("Query contains no columns with which to SELECT from.")
+            
         if eager_joins and self._should_nest_selectable:
             # for eager joins present and LIMIT/OFFSET/DISTINCT, wrap the query inside a select,
             # then append eager joins onto that

File lib/sqlalchemy/orm/session.py

             # remove associations
             cascaded = list(_cascade_state_iterator('refresh-expire', state))
             _expire_state(state, None)
-            for (state, m) in cascaded:
+            for (state, m, o) in cascaded:
                 _expire_state(state, None)
 
     def prune(self):
             raise sa_exc.InvalidRequestError(
                 "Instance %s is not present in this Session" %
                 mapperutil.state_str(state))
-        for s, m in [(state, None)] + list(_cascade_state_iterator('expunge', state)):
+        for s, m, o in [(state, None, None)] + list(_cascade_state_iterator('expunge', state)):
             self._expunge_state(s)
 
     def _expunge_state(self, state):
             state = attributes.instance_state(instance)
         except exc.NO_STATE:
             raise exc.UnmappedInstanceError(instance)
-        self._delete_impl(state)
-        for state, m in _cascade_state_iterator('delete', state):
+        
+        # grab the full cascade list first, since lazyloads/autoflush
+        # may be triggered by this operation (delete cascade lazyloads by default)
+        cascade_states = list(_cascade_state_iterator('delete', state))
+        self._delete_impl(state)    
+        for state, m, o in cascade_states:
             self._delete_impl(state, ignore_transient=True)
 
     def merge(self, instance, entity_name=None, dont_load=False,
 
 def _cascade_state_iterator(cascade, state, **kwargs):
     mapper = _state_mapper(state)
+    # yield the state, object, mapper.  yielding the object
+    # allows the iterator's results to be held in a list without
+    # states being garbage collected
     for (o, m) in mapper.cascade_iterator(cascade, state, **kwargs):
-        yield attributes.instance_state(o), m
+        yield attributes.instance_state(o), o, m
 
 def _cascade_unknown_state_iterator(cascade, state, **kwargs):
     mapper = _state_mapper(state)

File lib/sqlalchemy/orm/strategies.py

                 "lazy load operation of attribute '%s' cannot proceed" % 
                 (mapperutil.state_str(state), self.key)
             )
-
-        q = session.query(prop.mapper).autoflush(False)._adapt_all_clauses()
+        
+        q = session.query(prop.mapper)._adapt_all_clauses()
         
         if self.path:
             q = q._with_current_path(self.path)

File lib/sqlalchemy/sql/compiler.py

             if column.default is not None:
                 self.traverse_single(column.default)
 
-        self.append("\nCREATE " + " ".join(table._prefixes) + " TABLE " + self.preparer.format_table(table) + " (")
+        self.append("\nCREATE" + " ".join(table._prefixes) + " TABLE " + self.preparer.format_table(table) + " (")
 
         separator = "\n"
 

File test/orm/expire.py

 import testenv; testenv.configure_for_tests()
 import gc
 from testlib import sa, testing
-from testlib.sa import Table, Column, Integer, String, ForeignKey
+from testlib.sa import Table, Column, Integer, String, ForeignKey, exc as sa_exc
 from testlib.sa.orm import mapper, relation, create_session, attributes
 from orm import _base, _fixtures
 
         # but now its back, rollback has occured, the _remove_newly_deleted
         # is reverted
         self.assertEquals(u.name, 'chuck')
+    
+    @testing.resolve_artifact_names
+    def test_lazyload_autoflushes(self):
+        mapper(User, users, properties={
+            'addresses':relation(Address, order_by=addresses.c.email_address)
+        })
+        mapper(Address, addresses)
+        s = create_session(autoflush=True, autocommit=False)
+        u = s.query(User).get(8)
+        adlist = u.addresses
+        self.assertEquals(adlist, [
+            Address(email_address='ed@bettyboop.com'), 
+            Address(email_address='ed@lala.com'),
+            Address(email_address='ed@wood.com'), 
+        ])
+        a1 = u.addresses[2]
+        a1.email_address = 'aaaaa'
+        s.expire(u, ['addresses'])
+        self.assertEquals(u.addresses, [
+            Address(email_address='aaaaa'), 
+            Address(email_address='ed@bettyboop.com'), 
+            Address(email_address='ed@lala.com'),
+        ])
+
+    @testing.resolve_artifact_names
+    def test_refresh_collection_exception(self):
+        """test graceful failure for currently unsupported immediate refresh of a collection"""
+        
+        mapper(User, users, properties={
+            'addresses':relation(Address, order_by=addresses.c.email_address)
+        })
+        mapper(Address, addresses)
+        s = create_session(autoflush=True, autocommit=False)
+        u = s.query(User).get(8)
+        self.assertRaisesMessage(sa_exc.InvalidRequestError, "properties specified for refresh", s.refresh, u, ['addresses'])
+        
+        # in contrast to a regular query with no columns
+        self.assertRaisesMessage(sa_exc.InvalidRequestError, "no columns with which to SELECT", s.query().all)
         
     @testing.resolve_artifact_names
     def test_refresh_cancels_expire(self):

File test/orm/transaction.py

         s.add(u1)
         s.commit()
 
+        # this actually tests that the delete() operation,
+        # when cascaded to the "addresses" collection, does not
+        # trigger a flush (via lazyload) before the cascade is complete.
         s.delete(u1)
         assert u1 in s.deleted
         s.rollback()