- fixes [ticket:185], join object determines primary key and removes

columns that are FK's to other columns in the primary key collection.
- removed workaround code from query.py get()
- removed obsolete inheritance test from mapper
- added new get() test to inheritance.py for this particular issue
- ColumnCollection has nicer string method

3 files changed, 49 insertions, 27 deletions

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 070fb4cb5..77499c271 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -774,17 +774,9 @@ class Query(object):
             ident = key[1]
         else:
             ident = util.to_list(ident)
-        i = 0
         params = {}
-        for primary_key in self.primary_key_columns:
+        for i, primary_key in enumerate(self.primary_key_columns):
             params[primary_key._label] = ident[i]
-            # if there are not enough elements in the given identifier, then
-            # use the previous identifier repeatedly.  this is a workaround for the issue
-            # in [ticket:185], where a mapper that uses joined table inheritance needs to specify
-            # all primary keys of the joined relationship, which includes even if the join is joining
-            # two primary key (and therefore synonymous) columns together, the usual case for joined table inheritance.
-            if len(ident) > i + 1:
-                i += 1
         try:
             statement = self.compile(self._get_clause, lockmode=lockmode)
             return self._select_statement(statement, params=params, populate_existing=reload, version_check=(lockmode is not None))[0]
diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index bd601ed80..5ed95fabb 100644
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -680,7 +680,7 @@ class ForeignKey(SchemaItem):
         """Return True if the given table is referenced by this ``ForeignKey``."""
 
         return table.corresponding_column(self.column, False) is not None
-
+    
     def _init_column(self):
         # ForeignKey inits its remote column as late as possible, so tables can
         # be defined without dependencies
diff --git a/lib/sqlalchemy/sql.py b/lib/sqlalchemy/sql.py
index 6924a60ce..74f085cb1 100644
--- a/lib/sqlalchemy/sql.py
+++ b/lib/sqlalchemy/sql.py
@@ -1075,15 +1075,24 @@ class ColumnCollection(util.OrderedProperties):
         super(ColumnCollection, self).__init__()
         [self.add(c) for c in cols]
 
+    def __str__(self):
+        return repr([str(c) for c in self])
+        
     def add(self, column):
         """Add a column to this collection.
 
         The key attribute of the column will be used as the hash key
         for this dictionary.
         """
-
         self[column.key] = column
-
+    
+    def remove(self, column):
+        del self[column.key]
+        
+    def extend(self, iter):
+        for c in iter:
+            self.add(c)
+            
     def __eq__(self, other):
         l = []
         for c in other:
@@ -1243,6 +1252,16 @@ class FromClause(Selectable):
         self._primary_key = ColumnCollection()
         self._foreign_keys = util.Set()
         self._orig_cols = {}
+        for co in self._adjusted_exportable_columns():
+            cp = self._proxy_column(co)
+            for ci in cp.orig_set:
+                self._orig_cols[ci] = cp
+        if self.oid_column is not None:
+            for ci in self.oid_column.orig_set:
+                self._orig_cols[ci] = self.oid_column
+    
+    def _adjusted_exportable_columns(self):
+        """return the list of ColumnElements represented within this FromClause's _exportable_columns"""
         export = self._exportable_columns()
         for column in export:
             try:
@@ -1250,13 +1269,8 @@ class FromClause(Selectable):
             except AttributeError:
                 continue
             for co in s.columns:
-                cp = self._proxy_column(co)
-                for ci in cp.orig_set:
-                    self._orig_cols[ci] = cp
-        if self.oid_column is not None:
-            for ci in self.oid_column.orig_set:
-                self._orig_cols[ci] = self.oid_column
-
+                yield co
+        
     def _exportable_columns(self):
         return []
 
@@ -1661,10 +1675,23 @@ class Join(FromClause):
         else:
             self.onclause = onclause
         self.isouter = isouter
-
+        self.__folded_equivalents = None
+        self._init_primary_key()
+        
     name = property(lambda s: "Join object on " + s.left.name + " " + s.right.name)
     encodedname = property(lambda s: s.name.encode('ascii', 'backslashreplace'))
-    
+
+    def _init_primary_key(self):
+        pkcol = util.Set()
+        for col in self._adjusted_exportable_columns():
+            if col.primary_key:
+                pkcol.add(col)
+        for col in list(pkcol):
+            for f in col.foreign_keys:
+                if f.column in pkcol:
+                    pkcol.remove(col)
+        self.primary_key.extend(pkcol)
+        
     def _locate_oid_column(self):
         return self.left.oid_column
 
@@ -1673,8 +1700,6 @@ class Join(FromClause):
 
     def _proxy_column(self, column):
         self._columns[column._label] = column
-        if column.primary_key:
-            self._primary_key.add(column)
         for f in column.foreign_keys:
             self._foreign_keys.add(f)
         return column
@@ -1706,6 +1731,8 @@ class Join(FromClause):
         return True
 
     def _get_folded_equivalents(self, equivs=None):
+        if self.__folded_equivalents is not None:
+            return self.__folded_equivalents
         if equivs is None:
             equivs = util.Set()
         class LocateEquivs(NoColumnVisitor):
@@ -1731,7 +1758,8 @@ class Join(FromClause):
                     used.add(c.name)
             else: 
                 collist.append(c)
-        return collist
+        self.__folded_equivalents = collist
+        return self.__folded_equivalents
         
     def select(self, whereclause = None, fold_equivalents=False, **kwargs):
         """Create a ``Select`` from this ``Join``.
@@ -1740,9 +1768,11 @@ class Join(FromClause):
           the WHERE criterion that will be sent to the ``select()`` function
           
         fold_equivalents
-          based on the join criterion of this ``Join``, do not include equivalent
-          columns in the column list of the resulting select.  this will recursively
-          apply to any joins directly nested by this one as well.
+          based on the join criterion of this ``Join``, do not include repeat
+          column names in the column list of the resulting select, for columns that
+          are calculated to be "equivalent" based on the join criterion of this
+          ``Join``. this will recursively apply to any joins directly nested by
+          this one as well.
           
         \**kwargs
           all other kwargs are sent to the underlying ``select()`` function