Ticket #2874: select_related.2.patch

django/db/models/query.py

@@ -446 +446 @@
         where.extend(where2)
         params.extend(params2)
 
-        # Add additional tables and WHERE clauses based on select_related.
-        if self._select_related:
-            fill_table_cache(opts, select, tables, where, opts.db_table, [opts.db_table])
-
-        # Add any additional SELECTs.
-        if self._select:
-            select.extend(['(%s) AS %s' % (quote_only_if_word(s[1]), backend.quote_name(s[0])) for s in self._select.items()])
-
-        # Start composing the body of the SQL statement.
-        sql = [" FROM", backend.quote_name(opts.db_table)]
-
-        # Compose the join dictionary into SQL describing the joins.
-        if joins:
-            sql.append(" ".join(["%s %s AS %s ON %s" % (join_type, table, alias, condition)
-                            for (alias, (table, join_type, condition)) in joins.items()]))
-
-        # Compose the tables clause into SQL.
-        if tables:
-            sql.append(", " + ", ".join(tables))
-
-        # Compose the where clause into SQL.
-        if where:
-            sql.append(where and "WHERE " + " AND ".join(where))
-
-        # ORDER BY clause
+        # ORDER BY clause (this done before the other clauses since it is needed by follow_foreignkeys)
         order_by = []
+        ordering_tables = []
         if self._order_by is not None:
             ordering_to_use = self._order_by
         else:
@@ -488 +465 @@
                     order = "ASC"
                 if "." in col_name:
                     table_prefix, col_name = col_name.split('.', 1)
+                    ordering_tables.append(table_prefix)
                     table_prefix = backend.quote_name(table_prefix) + '.'
                 else:
                     # Use the database table as a column prefix if it wasn't given,
@@ -497 +475 @@
                     else:
                         table_prefix = ''
                 order_by.append('%s%s %s' % (table_prefix, backend.quote_name(orderfield2column(col_name, opts)), order))
+
+        # Add additional tables and WHERE clauses based on select_related.
+        if self._select_related:
+            more_select, more_tables, more_where = follow_foreignkeys(opts, [opts.db_table], ordering_tables)
+            select.extend(more_select)
+            tables.extend(more_tables)
+            where.extend(more_where)
+
+        # Add any additional SELECTs.
+        if self._select:
+            select.extend(['(%s) AS %s' % (quote_only_if_word(s[1]), backend.quote_name(s[0])) for s in self._select.items()])
+
+        # Start composing the body of the SQL statement.
+        sql = [" FROM", backend.quote_name(opts.db_table)]
+
+        # Compose the join dictionary into SQL describing the joins.
+        if joins:
+            sql.append(" ".join(["%s %s AS %s ON %s" % (join_type, table, alias, condition)
+                            for (alias, (table, join_type, condition)) in joins.items()]))
+
+        # Compose the tables clause into SQL.
+        if tables:
+            sql.append(", " + ", ".join(tables))
+
+        # Compose the where clause into SQL.
+        if where:
+            sql.append(where and "WHERE " + " AND ".join(where))
+
+        # finish up ORDER BY clause
         if order_by:
             sql.append("ORDER BY " + ", ".join(order_by))
 
@@ -662 +669 @@
             setattr(obj, f.get_cache_name(), rel_obj)
     return obj, index_end
 
-def fill_table_cache(opts, select, tables, where, old_prefix, cache_tables_seen):
-    """
-    Helper function that recursively populates the select, tables and where (in
-    place) for select_related queries.
-    """
+def follow_foreignkeys(object_to_follow, tables_seen, ordering_tables, recursion_level=0):
+    """Helper function that recursively follows ForeignKey fields in object_to_follow and returns
+    the approriate select, tables, and where lists for select_related queries"""
+    select = []
+    tables = []
+    where = []
     qn = backend.quote_name
-    for f in opts.fields:
-        if f.rel and not f.null:
-            db_table = f.rel.to._meta.db_table
-            if db_table not in cache_tables_seen:
-                tables.append(qn(db_table))
-            else: # The table was already seen, so give it a table alias.
-                new_prefix = '%s%s' % (db_table, len(cache_tables_seen))
-                tables.append('%s %s' % (qn(db_table), qn(new_prefix)))
-                db_table = new_prefix
-            cache_tables_seen.append(db_table)
+    for field in object_to_follow.fields:
+        if field.rel and not field.null:
+            related_table = field.rel.to._meta.db_table
+            if related_table == tables_seen[0]:
+                #stop if we follow a circular foreignkey relationship chain
+                return select, tables, where
+            if (related_table not in tables_seen) and ((related_table not in ordering_tables) or (recursion_level == 0)):
+                tables.append(qn(related_table))
+            else:
+                table_alias = '%s%s' % (related_table, len(tables_seen))
+                tables.append('%s %s' % (qn(related_table), qn(table_alias)))
+                related_table = table_alias
+            tables_seen.append(related_table)
             where.append('%s.%s = %s.%s' % \
-                (qn(old_prefix), qn(f.column), qn(db_table), qn(f.rel.get_related_field().column)))
-            select.extend(['%s.%s' % (qn(db_table), qn(f2.column)) for f2 in f.rel.to._meta.fields])
-            fill_table_cache(f.rel.to._meta, select, tables, where, db_table, cache_tables_seen)
+                (qn(object_to_follow.db_table), qn(field.column), qn(related_table), qn(field.rel.get_related_field().column)))
+            select.extend(['%s.%s' % (qn(related_table), qn(f2.column)) for f2 in field.rel.to._meta.fields])
+            more_select, more_tables, more_where = follow_foreignkeys(field.rel.to._meta, tables_seen, ordering_tables, recursion_level + 1)
+            select.extend(more_select)
+            tables.extend(more_tables)
+            where.extend(more_where)
+    return select, tables, where
 
 def parse_lookup(kwarg_items, opts):
     # Helper function that handles converting API kwargs