finally, a really straightforward reduce() method which reduces cols

to the minimal set for every test case I can come up with, and
now replaces all the cruft in Mapper._compile_pks() as well as
Join.__init_primary_key().  mappers can now handle aliased selects
and figure out the correct PKs pretty well [ticket:933]

2 files changed, 35 insertions, 54 deletions

diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index be870ee79..3ebc4960f 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -31,7 +31,7 @@ from sqlalchemy import util, exceptions
 from sqlalchemy.sql import operators, visitors
 from sqlalchemy import types as sqltypes
 
-functions, schema = None, None
+functions, schema, sql_util = None, None, None
 DefaultDialect, ClauseAdapter = None, None
 
 __all__ = [
@@ -2179,51 +2179,14 @@ class Join(FromClause):
 
         columns = list(self._flatten_exportable_columns())
 
-        #global sql_util
-        #if not sql_util:
-        #    from sqlalchemy.sql import util as sql_util
-        #self._primary_key = sql_util.reduce_columns([c for c in columns if c.primary_key], self.onclause)
-
-        self.__init_primary_key(columns)
+        global sql_util
+        if not sql_util:
+            from sqlalchemy.sql import util as sql_util
+        self._primary_key = sql_util.reduce_columns([c for c in columns if c.primary_key], self.onclause)
 
         for co in columns:
             cp = self._proxy_column(co)
 
-    def __init_primary_key(self, columns):
-        # TODO !!! remove all this
-        global schema
-        if schema is None:
-            from sqlalchemy import schema
-        pkcol = util.Set([c for c in columns if c.primary_key])
-
-        equivs = {}
-        def add_equiv(a, b):
-            for x, y in ((a, b), (b, a)):
-                if x in equivs:
-                    equivs[x].add(y)
-                else:
-                    equivs[x] = util.Set([y])
-
-        def visit_binary(binary):
-            if binary.operator == operators.eq and isinstance(binary.left, schema.Column) and isinstance(binary.right, schema.Column):
-                add_equiv(binary.left, binary.right)
-        visitors.traverse(self.onclause, visit_binary=visit_binary)
-
-        for col in pkcol:
-            for fk in col.foreign_keys:
-                if fk.column in pkcol:
-                    add_equiv(col, fk.column)
-
-        omit = util.Set()
-        for col in pkcol:
-            p = col
-            for c in equivs.get(col, util.Set()):
-                if p.references(c) or (c.primary_key and not p.primary_key):
-                    omit.add(p)
-                    p = c
-
-        self._primary_key = ColumnSet(pkcol.difference(omit))
-
     def description(self):
         return "Join object on %s(%d) and %s(%d)" % (self.left.description, id(self.left), self.right.description, id(self.right))
     description = property(description)
@@ -2284,6 +2247,12 @@ class Join(FromClause):
         """Returns the column list of this Join with all equivalently-named,
         equated columns folded into one column, where 'equated' means they are
         equated to each other in the ON clause of this join.
+        
+        this method is used by select(fold_equivalents=True).
+        
+        The primary usage for this is when generating UNIONs so that 
+        each selectable can have distinctly-named columns without the need
+        for use_labels=True.
         """
 
         if self.__folded_equivalents is not None:
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index 0989cb43e..93998c9a9 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -52,30 +52,42 @@ def find_columns(clause):
     
     
 def reduce_columns(columns, *clauses):
-    raise NotImplementedError()
+    """given a list of columns, return a 'reduced' set based on natural equivalents.
+
+    the set is reduced to the smallest list of columns which have no natural
+    equivalent present in the list.  A "natural equivalent" means that two columns
+    will ultimately represent the same value because they are related by a foreign key.
+    
+    \*clauses is an optional list of join clauses which will be traversed
+    to further identify columns that are "equivalent".
     
-    # TODO !!!
-    all_proxied_cols = util.Set(chain(*[c.proxy_set for c in columns]))
+    This function is primarily used to determine the most minimal "primary key"
+    from a selectable, by reducing the set of primary key columns present
+    in the the selectable to just those that are not repeated.
+    
+    """
     
     columns = util.Set(columns)
     
-    equivs = {}
+    omit = util.Set()
     for col in columns:
         for fk in col.foreign_keys:
-            if fk.column in all_proxied_cols:
-                for c in columns:
-                    if col.references(c):
-                        equivs[col] = c
+            for c in columns:
+                if c is col:
+                    continue
+                if fk.column.shares_lineage(c):
+                    omit.add(col)
+                    break
     
     if clauses:
         def visit_binary(binary):
-            if binary.operator == operators.eq and binary.left in columns and binary.right in columns:
-                equivs[binary.left] = binary.right
+            cols = columns.difference(omit)
+            if binary.operator == operators.eq and binary.left in cols and binary.right in cols:
+                omit.add(binary.right)
         for clause in clauses:
             visitors.traverse(clause, visit_binary=visit_binary)
     
-    result = util.Set([c for c in columns if c not in equivs])
-    return expression.ColumnSet(result)
+    return expression.ColumnSet(columns.difference(omit))
 
 class ColumnsInClause(visitors.ClauseVisitor):
     """Given a selectable, visit clauses and determine if any columns