From aa22cbd51aa6cb35fc658dd704948a18b27d1981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Sat, 15 Jun 2013 20:31:46 +0300 Subject: [PATCH] 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. --- django/db/models/sql/compiler.py | 19 +++++++------------ django/db/models/sql/query.py | 23 ++++++++++++----------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 0bfd1b38d3..77083372e0 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -631,12 +631,10 @@ class SQLCompiler(object): if not select_related_descend(f, restricted, requested, only_load.get(field_model)): continue - table = f.rel.to._meta.db_table promote = nullable or f.null - alias = self.query.join_parent_model(opts, model, root_alias, {}) - join_cols = f.get_joining_columns() - alias = self.query.join((alias, table, join_cols), - outer_if_first=promote, join_field=f) + _, _, _, joins, _ = self.query.setup_joins( + [f.name], opts, root_alias, outer_if_first=promote) + alias = joins[-1] columns, aliases = self.get_default_columns(start_alias=alias, opts=f.rel.to._meta, as_pairs=True) self.query.related_select_cols.extend( @@ -660,12 +658,9 @@ class SQLCompiler(object): only_load.get(model), reverse=True): continue - alias = self.query.join_parent_model(opts, f.rel.to, root_alias, {}) - table = model._meta.db_table - alias = self.query.join( - (alias, table, f.get_joining_columns(reverse_join=True)), - outer_if_first=True, join_field=f - ) + _, _, _, joins, _ = self.query.setup_joins( + [f.related_query_name()], opts, root_alias, outer_if_first=True) + alias = joins[-1] from_parent = (opts.model if issubclass(model, opts.model) else None) columns, aliases = self.get_default_columns(start_alias=alias, @@ -677,7 +672,7 @@ class SQLCompiler(object): # Use True here because we are looking at the _reverse_ side of # the relation, which is always nullable. new_nullable = True - + table = model._meta.db_table self.fill_related_selections(model._meta, table, cur_depth+1, next, restricted, new_nullable) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index d26b8d5c95..a56718c904 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -926,10 +926,10 @@ class Query(object): """ if model in seen: return seen[model] - int_opts = opts chain = opts.get_base_chain(model) if chain is None: return alias + curr_opts = opts for int_model in chain: if int_model in seen: return seen[int_model] @@ -937,14 +937,14 @@ class Query(object): # with no parents, assign the new options # object and skip to the next base in that # case - if not int_opts.parents[int_model]: - int_opts = int_model._meta + if not curr_opts.parents[int_model]: + curr_opts = int_model._meta continue - link_field = int_opts.get_ancestor_link(int_model) - int_opts = int_model._meta - connection = (alias, int_opts.db_table, link_field.get_joining_columns()) - alias = seen[int_model] = self.join(connection, nullable=False, - join_field=link_field) + link_field = curr_opts.get_ancestor_link(int_model) + _, _, _, joins, _ = self.setup_joins( + [link_field.name], curr_opts, alias) + curr_opts = int_model._meta + alias = seen[int_model] = joins[-1] return alias or seen[None] def remove_inherited_models(self): @@ -1321,7 +1321,7 @@ class Query(object): return path, final_field, targets def setup_joins(self, names, opts, alias, can_reuse=None, allow_many=True, - allow_explicit_fk=False): + allow_explicit_fk=False, outer_if_first=False): """ Compute the necessary table joins for the passage through the fields given in 'names'. 'opts' is the Options class for the current model @@ -1364,8 +1364,9 @@ class Query(object): nullable = True connection = alias, opts.db_table, join.join_field.get_joining_columns() reuse = can_reuse if join.m2m else None - alias = self.join(connection, reuse=reuse, - nullable=nullable, join_field=join.join_field) + alias = self.join( + connection, reuse=reuse, nullable=nullable, join_field=join.join_field, + outer_if_first=outer_if_first) joins.append(alias) if hasattr(final_field, 'field'): final_field = final_field.field