- fixed endless loop bug which could occur

within a mapper's deferred load of
inherited attributes.
- declarative initialization of Columns adjusted so that
non-renamed columns initialize in the same way as a non
declarative mapper.   This allows an inheriting mapper
to set up its same-named "id" columns in particular
such that the parent "id" column is favored over the child
column, reducing database round trips when this value
is requested.

4 files changed, 38 insertions, 19 deletions

diff --git a/lib/sqlalchemy/ext/declarative.py b/lib/sqlalchemy/ext/declarative.py
index 47ee427dd..65af2da7a 100644
--- a/lib/sqlalchemy/ext/declarative.py
+++ b/lib/sqlalchemy/ext/declarative.py
@@ -270,6 +270,13 @@ def _as_declarative(cls, classname, dict_):
                 elif isinstance(c, Column):
                     _undefer_column_name(key, c)
                     cols.append(c)
+                    # if the column is the same name as the key, 
+                    # remove it from the explicit properties dict.
+                    # the normal rules for assigning column-based properties
+                    # will take over, including precedence of columns
+                    # in multi-column ColumnProperties.
+                    if key == c.key:
+                        del our_stuff[key]
             cls.__table__ = table = Table(tablename, cls.metadata,
                                           *(tuple(cols) + tuple(args)), **table_kw)
     else:
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 9b088aae7..590424759 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -339,7 +339,7 @@ class AttributeImpl(object):
     def set(self, state, value, initiator):
         raise NotImplementedError()
 
-    def get_committed_value(self, state):
+    def get_committed_value(self, state, passive=False):
         """return the unchanged value of this attribute"""
 
         if self.key in state.committed_state:
@@ -348,7 +348,7 @@ class AttributeImpl(object):
             else:
                 return state.committed_state.get(self.key)
         else:
-            return self.get(state)
+            return self.get(state, passive=passive)
 
     def set_committed_value(self, state, value):
         """set an attribute value on the given instance and 'commit' it."""
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 41908e832..732e65fb7 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1009,8 +1009,8 @@ class Mapper(object):
         state = attributes.instance_state(obj)
         return self._get_committed_state_attr_by_column(state, column)
 
-    def _get_committed_state_attr_by_column(self, state, column):
-        return self._get_col_to_prop(column).getcommitted(state, column)
+    def _get_committed_state_attr_by_column(self, state, column, passive=False):
+        return self._get_col_to_prop(column).getcommitted(state, column, passive=passive)
 
     def _save_obj(self, states, uowtransaction, postupdate=False, post_update_cols=None, single=False):
         """Issue ``INSERT`` and/or ``UPDATE`` statements for a list of objects.
@@ -1458,7 +1458,7 @@ class Mapper(object):
                     identitykey = self._identity_key_from_state(refresh_state)
             else:
                 identitykey = identity_key(row)
-
+            
             if identitykey in session_identity_map:
                 instance = session_identity_map[identitykey]
                 state = attributes.instance_state(instance)
@@ -1582,6 +1582,9 @@ class Mapper(object):
         if self.base_mapper.local_table in tables:
             return None
 
+        class ColumnsNotAvailable(Exception):
+            pass
+            
         def visit_binary(binary):
             leftcol = binary.left
             rightcol = binary.right
@@ -1589,19 +1592,28 @@ class Mapper(object):
                 return
 
             if leftcol.table not in tables:
-                binary.left = sql.bindparam(None, self._get_committed_state_attr_by_column(state, leftcol), type_=binary.right.type)
+                leftval = self._get_committed_state_attr_by_column(state, leftcol, passive=True)
+                if leftval is attributes.PASSIVE_NORESULT:
+                    raise ColumnsNotAvailable()
+                binary.left = sql.bindparam(None, leftval, type_=binary.right.type)
             elif rightcol.table not in tables:
-                binary.right = sql.bindparam(None, self._get_committed_state_attr_by_column(state, rightcol), type_=binary.right.type)
+                rightval = self._get_committed_state_attr_by_column(state, rightcol, passive=True)
+                if rightval is attributes.PASSIVE_NORESULT:
+                    raise ColumnsNotAvailable()
+                binary.right = sql.bindparam(None, rightval, type_=binary.right.type)
 
         allconds = []
 
-        start = False
-        for mapper in reversed(list(self.iterate_to_root())):
-            if mapper.local_table in tables:
-                start = True
-            if start and not mapper.single:
-                allconds.append(visitors.cloned_traverse(mapper.inherit_condition, {}, {'binary':visit_binary}))
-
+        try:
+            start = False
+            for mapper in reversed(list(self.iterate_to_root())):
+                if mapper.local_table in tables:
+                    start = True
+                if start and not mapper.single:
+                    allconds.append(visitors.cloned_traverse(mapper.inherit_condition, {}, {'binary':visit_binary}))
+        except ColumnsNotAvailable:
+            return None
+            
         cond = sql.and_(*allconds)
         return sql.select(tables, cond, use_labels=True)
 
@@ -1638,7 +1650,7 @@ def _load_scalar_attributes(state, attribute_names):
         raise sa_exc.UnboundExecutionError("Instance %s is not bound to a Session; attribute refresh operation cannot proceed" % (state_str(state)))
 
     has_key = _state_has_identity(state)
-
+    
     result = False
     if mapper.inherits and not mapper.concrete:
         statement = mapper._optimized_get_statement(state, attribute_names)
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index d6e6b1f3c..5eec3ce3f 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -64,8 +64,8 @@ class ColumnProperty(StrategizedProperty):
     def getattr(self, state, column):
         return state.get_impl(self.key).get(state)
 
-    def getcommitted(self, state, column):
-        return state.get_impl(self.key).get_committed_value(state)
+    def getcommitted(self, state, column, passive=False):
+        return state.get_impl(self.key).get_committed_value(state, passive=passive)
 
     def setattr(self, state, value, column):
         state.get_impl(self.key).set(state, value, None)
@@ -118,8 +118,8 @@ class CompositeProperty(ColumnProperty):
         obj = state.get_impl(self.key).get(state)
         return self.get_col_value(column, obj)
 
-    def getcommitted(self, state, column):
-        obj = state.get_impl(self.key).get_committed_value(state)
+    def getcommitted(self, state, column, passive=False):
+        obj = state.get_impl(self.key).get_committed_value(state, passive=passive)
         return self.get_col_value(column, obj)
 
     def setattr(self, state, value, column):