Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #20583 -- ORM always uses setup_joins() for join generation

There were a couple of places which used Query.join() directly. By
using setup_joins() in these places the code is more DRY, and in
addition there is no need to directly call field.get_joining_columns()
unless the field is the given join_field from get_path_info(). This
makes it easier to make sure a ForeignObject subclass generates joins
correctly in all cases.
  • Loading branch information...
commit aa22cbd51aa6cb35fc658dd704948a18b27d1981 1 parent b3532f7
Anssi Kääriäinen authored June 15, 2013
19  django/db/models/sql/compiler.py
@@ -631,12 +631,10 @@ def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
631 631
             if not select_related_descend(f, restricted, requested,
632 632
                                           only_load.get(field_model)):
633 633
                 continue
634  
-            table = f.rel.to._meta.db_table
635 634
             promote = nullable or f.null
636  
-            alias = self.query.join_parent_model(opts, model, root_alias, {})
637  
-            join_cols = f.get_joining_columns()
638  
-            alias = self.query.join((alias, table, join_cols),
639  
-                    outer_if_first=promote, join_field=f)
  635
+            _, _, _, joins, _ = self.query.setup_joins(
  636
+                [f.name], opts, root_alias, outer_if_first=promote)
  637
+            alias = joins[-1]
640 638
             columns, aliases = self.get_default_columns(start_alias=alias,
641 639
                     opts=f.rel.to._meta, as_pairs=True)
642 640
             self.query.related_select_cols.extend(
@@ -660,12 +658,9 @@ def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
660 658
                                               only_load.get(model), reverse=True):
661 659
                     continue
662 660
 
663  
-                alias = self.query.join_parent_model(opts, f.rel.to, root_alias, {})
664  
-                table = model._meta.db_table
665  
-                alias = self.query.join(
666  
-                    (alias, table, f.get_joining_columns(reverse_join=True)),
667  
-                    outer_if_first=True, join_field=f
668  
-                )
  661
+                _, _, _, joins, _ = self.query.setup_joins(
  662
+                    [f.related_query_name()], opts, root_alias, outer_if_first=True)
  663
+                alias = joins[-1]
669 664
                 from_parent = (opts.model if issubclass(model, opts.model)
670 665
                                else None)
671 666
                 columns, aliases = self.get_default_columns(start_alias=alias,
@@ -677,7 +672,7 @@ def fill_related_selections(self, opts=None, root_alias=None, cur_depth=1,
677 672
                 # Use True here because we are looking at the _reverse_ side of
678 673
                 # the relation, which is always nullable.
679 674
                 new_nullable = True
680  
-
  675
+                table = model._meta.db_table
681 676
                 self.fill_related_selections(model._meta, table, cur_depth+1,
682 677
                     next, restricted, new_nullable)
683 678
 
23  django/db/models/sql/query.py
@@ -926,10 +926,10 @@ def join_parent_model(self, opts, model, alias, seen):
926 926
         """
927 927
         if model in seen:
928 928
             return seen[model]
929  
-        int_opts = opts
930 929
         chain = opts.get_base_chain(model)
931 930
         if chain is None:
932 931
             return alias
  932
+        curr_opts = opts
933 933
         for int_model in chain:
934 934
             if int_model in seen:
935 935
                 return seen[int_model]
@@ -937,14 +937,14 @@ def join_parent_model(self, opts, model, alias, seen):
937 937
             # with no parents, assign the new options
938 938
             # object and skip to the next base in that
939 939
             # case
940  
-            if not int_opts.parents[int_model]:
941  
-                int_opts = int_model._meta
  940
+            if not curr_opts.parents[int_model]:
  941
+                curr_opts = int_model._meta
942 942
                 continue
943  
-            link_field = int_opts.get_ancestor_link(int_model)
944  
-            int_opts = int_model._meta
945  
-            connection = (alias, int_opts.db_table, link_field.get_joining_columns())
946  
-            alias = seen[int_model] = self.join(connection, nullable=False,
947  
-                                                join_field=link_field)
  943
+            link_field = curr_opts.get_ancestor_link(int_model)
  944
+            _, _, _, joins, _ = self.setup_joins(
  945
+                [link_field.name], curr_opts, alias)
  946
+            curr_opts = int_model._meta
  947
+            alias = seen[int_model] = joins[-1]
948 948
         return alias or seen[None]
949 949
 
950 950
     def remove_inherited_models(self):
@@ -1321,7 +1321,7 @@ def names_to_path(self, names, opts, allow_many, allow_explicit_fk):
1321 1321
         return path, final_field, targets
1322 1322
 
1323 1323
     def setup_joins(self, names, opts, alias, can_reuse=None, allow_many=True,
1324  
-                    allow_explicit_fk=False):
  1324
+                    allow_explicit_fk=False, outer_if_first=False):
1325 1325
         """
1326 1326
         Compute the necessary table joins for the passage through the fields
1327 1327
         given in 'names'. 'opts' is the Options class for the current model
@@ -1364,8 +1364,9 @@ def setup_joins(self, names, opts, alias, can_reuse=None, allow_many=True,
1364 1364
                 nullable = True
1365 1365
             connection = alias, opts.db_table, join.join_field.get_joining_columns()
1366 1366
             reuse = can_reuse if join.m2m else None
1367  
-            alias = self.join(connection, reuse=reuse,
1368  
-                              nullable=nullable, join_field=join.join_field)
  1367
+            alias = self.join(
  1368
+                connection, reuse=reuse, nullable=nullable, join_field=join.join_field,
  1369
+                outer_if_first=outer_if_first)
1369 1370
             joins.append(alias)
1370 1371
         if hasattr(final_field, 'field'):
1371 1372
             final_field = final_field.field

0 notes on commit aa22cbd

Please sign in to comment.
Something went wrong with that request. Please try again.