Code

Ticket #1136: unique_joins.patch

File unique_joins.patch, 15.3 KB (added by freakboy@…, 9 years ago)

Patch to remove duplicated joins on kwarg evaluation

  • django/db/models/manager.py

     
    6161 
    6262        # Construct the fundamental parts of the query: SELECT X FROM Y WHERE Z. 
    6363        select = ["%s.%s" % (backend.quote_name(opts.db_table), backend.quote_name(f.column)) for f in opts.fields] 
    64         tables = [opts.db_table] + (kwargs.get('tables') and kwargs['tables'][:] or []) 
    65         tables = [quote_only_if_word(t) for t in tables] 
     64        tables = (kwargs.get('tables') and [quote_only_if_word(t) for t in kwargs['tables']] or []) 
    6665        where = kwargs.get('where') and kwargs['where'][:] or [] 
    6766        params = kwargs.get('params') and kwargs['params'][:] or [] 
    6867 
    6968        # Convert the kwargs into SQL. 
    70         tables2, join_where2, where2, params2, _ = parse_lookup(kwargs.items(), opts) 
     69        tables2, joins, where2, params2 = parse_lookup(kwargs.items(), opts) 
    7170        tables.extend(tables2) 
    72         where.extend(join_where2 + where2) 
     71        where.extend(where2) 
    7372        params.extend(params2) 
    7473 
    7574        # Add any additional constraints from the "where_constraints" parameter. 
     
    8382        if kwargs.get('select'): 
    8483            select.extend(['(%s) AS %s' % (quote_only_if_word(s[1]), backend.quote_name(s[0])) for s in kwargs['select']]) 
    8584 
     85        # Start composing the body of the SQL statement 
     86        sql = [" FROM", backend.quote_name(opts.db_table)] 
     87 
     88        # Compose the join dictionary into SQL describing the joins 
     89        if joins:  
     90            sql.append(" ".join(["%s %s AS %s ON %s" % (join_type, table, alias, condition)  
     91                                for (alias, (table, join_type, condition)) in joins.items()])) 
     92 
     93        # Compose the tables clause into SQL 
     94        if tables:  
     95            sql.append(", " + ", ".join(tables)) 
     96 
     97        # Compose the where clause into SQL 
     98        if where:  
     99            sql.append(where and "WHERE " + " AND ".join(where)) 
     100 
    86101        # ORDER BY clause 
    87102        order_by = [] 
    88103        for f in handle_legacy_orderlist(kwargs.get('order_by', opts.ordering)): 
     
    106121                    else: 
    107122                        table_prefix = '' 
    108123                order_by.append('%s%s %s' % (table_prefix, backend.quote_name(orderfield2column(col_name, opts)), order)) 
    109         order_by = ", ".join(order_by) 
     124        if order_by: 
     125            sql.append("ORDER BY " + ", ".join(order_by)) 
    110126 
    111127        # LIMIT and OFFSET clauses 
    112128        if kwargs.get('limit') is not None: 
    113             limit_sql = " %s " % backend.get_limit_offset_sql(kwargs['limit'], kwargs.get('offset')) 
     129            sql.append("%s " % backend.get_limit_offset_sql(kwargs['limit'], kwargs.get('offset'))) 
    114130        else: 
    115131            assert kwargs.get('offset') is None, "'offset' is not allowed without 'limit'" 
    116             limit_sql = "" 
    117132 
    118         return select, " FROM " + ",".join(tables) + (where and " WHERE " + " AND ".join(where) or "") + (order_by and " ORDER BY " + order_by or "") + limit_sql, params 
    119  
     133        return select, " ".join(sql), params 
     134         
    120135    def get_iterator(self, **kwargs): 
    121136        # kwargs['select'] is a dictionary, and dictionaries' key order is 
    122137        # undefined, so we convert it to a list of tuples internally. 
  • django/db/models/query.py

     
    4545            output.append('%s%s ASC' % (prefix, backend.quote_name(orderfield2column(f, opts)))) 
    4646    return ', '.join(output) 
    4747 
    48 class QBase: 
     48class QOperator: 
    4949    "Base class for QAnd and QOr" 
    5050    def __init__(self, *args): 
    5151        self.args = args 
     
    5353    def __repr__(self): 
    5454        return '(%s)' % self.operator.join([repr(el) for el in self.args]) 
    5555 
    56     def get_sql(self, opts, table_count): 
    57         tables, join_where, where, params = [], [], [], [] 
     56    def get_sql(self, opts): 
     57        tables, joins, where, params = [], {}, [], [] 
    5858        for val in self.args: 
    59             tables2, join_where2, where2, params2, table_count = val.get_sql(opts, table_count) 
     59            tables2, joins2, where2, params2 = val.get_sql(opts) 
    6060            tables.extend(tables2) 
    61             join_where.extend(join_where2) 
     61            joins.update(joins2) 
    6262            where.extend(where2) 
    6363            params.extend(params2) 
    64         return tables, join_where, ['(%s)' % self.operator.join(where)], params, table_count 
     64        return tables, joins, ['(%s)' % self.operator.join(where)], params 
    6565 
    66 class QAnd(QBase): 
     66class QAnd(QOperator): 
    6767    "Encapsulates a combined query that uses 'AND'." 
    6868    operator = ' AND ' 
    6969    def __or__(self, other): 
     
    8080        else: 
    8181            raise TypeError, other 
    8282 
    83 class QOr(QBase): 
     83class QOr(QOperator): 
    8484    "Encapsulates a combined query that uses 'OR'." 
    8585    operator = ' OR ' 
    8686    def __and__(self, other): 
     
    117117        else: 
    118118            raise TypeError, other 
    119119 
    120     def get_sql(self, opts, table_count): 
    121         return parse_lookup(self.kwargs.items(), opts, table_count) 
     120    def get_sql(self, opts): 
     121        return parse_lookup(self.kwargs.items(), opts) 
    122122 
    123123 
    124124def get_where_clause(lookup_type, table_prefix, field_name, value): 
     
    174174    # Helper function to remove redundancy. 
    175175    raise TypeError, "got unexpected keyword argument '%s'" % kwarg 
    176176 
    177 def parse_lookup(kwarg_items, opts, table_count=0): 
     177def parse_lookup(kwarg_items, opts): 
    178178    # Helper function that handles converting API kwargs (e.g. 
    179179    # "name__exact": "tom") to SQL. 
    180180 
    181     # Note that there is a distinction between where and join_where. The latter 
    182     # is specifically a list of where clauses to use for JOINs. This 
    183     # distinction is necessary because of support for "_or". 
    184  
    185     # table_count is used to ensure table aliases are unique. 
    186     tables, join_where, where, params = [], [], [], [] 
     181    # 'joins' is a dictionary describing the tables that must be joined to complete the query. 
     182    # Each key-value pair follows the form 
     183    #   alias: (table, join_type, condition) 
     184    # where 
     185    #   alias is the AS alias for the joined table 
     186    #   table is the actual table name to be joined 
     187    #   join_type is the type of join (INNER JOIN, LEFT OUTER JOIN, etc)  
     188    #   condition is the where-like statement over which narrows the join. 
     189    # 
     190    # alias will be derived from the lookup list name. 
     191    # At present, this method only every returns INNER JOINs; the option is there for others 
     192    # to implement custom Q()s, etc that return other join types. 
     193    tables, joins, where, params = [], {}, [], [] 
    187194    for kwarg, kwarg_value in kwarg_items: 
    188195        if kwarg in ('order_by', 'limit', 'offset', 'select_related', 'distinct', 'select', 'tables', 'where', 'params'): 
    189196            continue 
    190197        if kwarg_value is None: 
    191198            continue 
    192199        if kwarg == 'complex': 
    193             tables2, join_where2, where2, params2, table_count = kwarg_value.get_sql(opts, table_count) 
     200            tables2, joins2, where2, params2 = kwarg_value.get_sql(opts) 
    194201            tables.extend(tables2) 
    195             join_where.extend(join_where2) 
     202            joins.update(joins2) 
    196203            where.extend(where2) 
    197             params.extend(params2) 
     204            params.extend(params2)             
    198205            continue 
    199206        if kwarg == '_or': 
    200207            for val in kwarg_value: 
    201                 tables2, join_where2, where2, params2, table_count = parse_lookup(val, opts, table_count) 
     208                tables2, joins2, where2, params2 = parse_lookup(val, opts) 
    202209                tables.extend(tables2) 
    203                 join_where.extend(join_where2) 
     210                joins.update(joins2) 
    204211                where.append('(%s)' % ' OR '.join(where2)) 
    205212                params.extend(params2) 
    206213            continue 
     
    212219            else: 
    213220                lookup_list = lookup_list[:-1] + [opts.pk.name, 'exact'] 
    214221        if len(lookup_list) == 1: 
    215             _throw_bad_kwarg_error(kwarg) 
     222            throw_bad_kwarg_error(kwarg) 
    216223        lookup_type = lookup_list.pop() 
    217224        current_opts = opts # We'll be overwriting this, so keep a reference to the original opts. 
    218225        current_table_alias = current_opts.db_table 
    219226        param_required = False 
    220227        while lookup_list or param_required: 
    221             table_count += 1 
    222228            try: 
    223229                # "current" is a piece of the lookup list. For example, in 
    224230                # choices.get_list(poll__sites__id__exact=5), lookup_list is 
    225                 # ["polls", "sites", "id"], and the first current is "polls". 
     231                # ["poll", "sites", "id"], and the first current is "poll". 
    226232                try: 
    227233                    current = lookup_list.pop(0) 
    228234                except IndexError: 
     
    233239                # Try many-to-many relationships first... 
    234240                for f in current_opts.many_to_many: 
    235241                    if f.name == current: 
    236                         rel_table_alias = backend.quote_name('t%s' % table_count) 
    237                         table_count += 1 
    238                         tables.append('%s %s' % \ 
    239                             (backend.quote_name(f.get_m2m_db_table(current_opts)), rel_table_alias)) 
    240                         join_where.append('%s.%s = %s.%s' % \ 
    241                             (backend.quote_name(current_table_alias), 
    242                             backend.quote_name(current_opts.pk.column), 
    243                             rel_table_alias, 
    244                             backend.quote_name(current_opts.object_name.lower() + '_id'))) 
     242                        rel_table_alias = backend.quote_name(current_table_alias + LOOKUP_SEPARATOR + current) 
     243             
     244                        joins[rel_table_alias] = ( 
     245                            backend.quote_name(f.get_m2m_db_table(current_opts)),  
     246                            "INNER JOIN",  
     247                            '%s.%s = %s.%s' %  
     248                                (backend.quote_name(current_table_alias), 
     249                                backend.quote_name(current_opts.pk.column), 
     250                                rel_table_alias, 
     251                                backend.quote_name(current_opts.object_name.lower() + '_id')) 
     252                        ) 
     253 
    245254                        # Optimization: In the case of primary-key lookups, we 
    246255                        # don't have to do an extra join. 
    247256                        if lookup_list and lookup_list[0] == f.rel.to._meta.pk.name and lookup_type == 'exact': 
     
    251260                            lookup_list.pop() 
    252261                            param_required = False 
    253262                        else: 
    254                             new_table_alias = 't%s' % table_count 
    255                             tables.append('%s %s' % (backend.quote_name(f.rel.to._meta.db_table), 
    256                                 backend.quote_name(new_table_alias))) 
    257                             join_where.append('%s.%s = %s.%s' % \ 
    258                                 (backend.quote_name(rel_table_alias), 
    259                                 backend.quote_name(f.rel.to._meta.object_name.lower() + '_id'), 
    260                                 backend.quote_name(new_table_alias), 
    261                                 backend.quote_name(f.rel.to._meta.pk.column))) 
     263                            new_table_alias = current_table_alias + LOOKUP_SEPARATOR + current 
     264 
     265                            joins[backend.quote_name(new_table_alias)] = ( 
     266                                backend.quote_name(f.rel.to._meta.db_table), 
     267                                "INNER JOIN", 
     268                                '%s.%s = %s.%s' %  
     269                                    (rel_table_alias, 
     270                                    backend.quote_name(f.rel.to._meta.object_name.lower() + '_id'), 
     271                                    backend.quote_name(new_table_alias), 
     272                                    backend.quote_name(f.rel.to._meta.pk.column)) 
     273                            ) 
    262274                            current_table_alias = new_table_alias 
    263275                            param_required = True 
    264276                        current_opts = f.rel.to._meta 
     
    279291                        elif lookup_type == 'isnull' and not lookup_list: 
    280292                            where.append(get_where_clause(lookup_type, current_table_alias+'.', f.column, kwarg_value)) 
    281293                            params.extend(f.get_db_prep_lookup(lookup_type, kwarg_value)) 
    282                         else: 
    283                             new_table_alias = 't%s' % table_count 
    284                             tables.append('%s %s' % \ 
    285                                 (backend.quote_name(f.rel.to._meta.db_table), backend.quote_name(new_table_alias))) 
    286                             join_where.append('%s.%s = %s.%s' % \ 
    287                                 (backend.quote_name(current_table_alias), backend.quote_name(f.column), 
    288                                 backend.quote_name(new_table_alias), backend.quote_name(f.rel.to._meta.pk.column))) 
     294                        else:                             
     295                            new_table_alias = current_table_alias + LOOKUP_SEPARATOR + current 
     296                             
     297                            joins[backend.quote_name(new_table_alias)] = ( 
     298                                backend.quote_name(f.rel.to._meta.db_table), 
     299                                "INNER JOIN", 
     300                                '%s.%s = %s.%s' %  
     301                                    (backend.quote_name(current_table_alias), 
     302                                    backend.quote_name(f.column), 
     303                                    backend.quote_name(new_table_alias),  
     304                                    backend.quote_name(f.rel.to._meta.pk.column)) 
     305                            ) 
    289306                            current_table_alias = new_table_alias 
    290307                            param_required = True 
    291308                        current_opts = f.rel.to._meta 
     
    301318                throw_bad_kwarg_error(kwarg) 
    302319            except StopIteration: 
    303320                continue 
    304     return tables, join_where, where, params, table_count 
     321    return tables, joins, where, params 
  • tests/modeltests/many_to_one/models.py

     
    6767>>> Article.objects.get_list(reporter__first_name__exact='John', order_by=['pub_date']) 
    6868[This is a test, John's second story] 
    6969 
     70# Query twice over the related field 
     71>>> Article.objects.get_list(reporter__first_name__exact='John', reporter__last_name__exact='Smith') 
     72[This is a test, John's second story] 
     73 
     74# The underlying query only makes one join when a related table is referenced twice 
     75>>> _,sql,_ = Article.objects._get_sql_clause(reporter__first_name__exact='John', reporter__last_name__exact='Smith') 
     76>>> sql.count('INNER JOIN') 
     771 
     78 
     79# The automatically joined table has a predictable name 
     80>>> Article.objects.get_list(reporter__first_name__exact='John', where=["many_to_one_articles__reporter.last_name='Smith'"]) 
     81[This is a test, John's second story] 
     82 
    7083# Find all Articles for the Reporter whose ID is 1. 
    7184>>> Article.objects.get_list(reporter__id__exact=1, order_by=['pub_date']) 
    7285[This is a test, John's second story] 
     
    95108>>> a4.save() 
    96109>>> a4.get_reporter() 
    97110John Smith 
     111 
    98112"""