Code

Opened 6 years ago

Closed 6 years ago

#6704 closed (fixed)

.exclude(foreignkey_field__isnull=True) doesn't quite work on qs-rf

Reported by: matt@… Owned by: nobody
Component: Uncategorized Version: queryset-refactor
Severity: Keywords: qs-rf
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

It gets turned into: (NOT table.foreignkey_field_id IS NULL OR table.foreignkey_field_id IS NULL) which of course is always going to be true :)

Haven't got my head around the code yet, but I'm guessing the following line is being run in this situation when it shouldn't be (I presume the line is intended for when a join is being done to get the required attribute on the related object, but for the special case of the id/pk field no join is needed and in fact no join was added):

self.where.add([alias, col, field, 'isnull', True], OR)

Attachments (0)

Change History (3)

comment:1 Changed 6 years ago by matt@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I still haven't really got my head around the query stuff, but looking through a bit more I see where the optimisation is happening to remove the join 'cos it's the key field...

        if join_list: 
            # An optimization: if the final join is against the same column as
            # we are comparing against, we can go back one step in the join
            # chain and compare against the lhs of the join instead. The result
            # (potentially) involves one less table join.
            join = self.alias_map[alias][ALIAS_JOIN]
            if col == join[RHS_JOIN_COL]:
                self.unref_alias(alias)
                alias = join[LHS_ALIAS]
                col = join[LHS_JOIN_COL]

Does it matter that this code isn't also removing the join info from join_list (as join_list is processed by subsequent code in the function so)?

Anyway my guess at the fix (and it is a bit of a guess) for the exclude is_null bug is to tweak this bit of code to not fire if the lookup_type is "isnull" and value is True, although from the comment it may be Work In Progress anyway:

            if flag:
                # XXX: Change this to the field we joined against to allow
                # for node sharing and where-tree optimisation?
                self.where.add([alias, col, field, 'isnull', True], OR)

comment:2 Changed 6 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Accepted

You can stop guessing. I'm working on this.

It is work in progress, so just bear with me for a little bit. A bunch of that code is being slightly rewritten to fix some other bugs, so this will be fixed in passing.

comment:3 Changed 6 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [7217]) queryset-refactor: Reworked exclude() handling to fix a few merging problems.

Fixed #6704.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.