Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#6986 closed (fixed)

Multiple conditions in Q objects are connected with differrent operators, depending on context

Reported by: emes Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qsrf-cleanup Q, operator, context
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The example query is:

q = Playteam.objects.filter(
    Q(moderator=request.user) |
    Q(
        playteammembership__user=request.user,
        playteammembership__user_accepted=True,
        playteammembership__playteam_accepted=True
      )
    ).order_by('name')

The query I intended to make should return a list of Playteam objects where request.user is a moderator or a member with a membership object having confirmation from both sides (half-confirmed membership objects act as invitations).

The SQL result is a big surprise:

In [64]: p._get_sql_clause()
Out[64]: 
(['"playarena_playteam"."id"',
  '"playarena_playteam"."name"',
  '"playarena_playteam"."slug"',
  '"playarena_playteam"."description"',
  '"playarena_playteam"."city"',
  '"playarena_playteam"."province"',
  '"playarena_playteam"."moderator_id"',
  '"playarena_playteam"."active"',
  '"playarena_playteam"."players_wanted"'],
 ' FROM "playarena_playteam" INNER JOIN "playarena_playteammembership" AS "playarena_playteam__playteammembership" ON "playarena_playteam"."id" = "playarena_playteam__playteammembership"."playteam_id" WHERE (("playarena_playteam"."moderator_id" = %s OR "playarena_playteam__playteammembership"."user_id" = %s OR "playarena_playteam__playteammembership"."user_accepted" = %s OR "playarena_playteam__playteammembership"."playteam_accepted" = %s))',
 [1, 1, True, True])

All conditions in second Q have been connected with OR operator.

What't more interesting:

q = Playteam.objects.filter(
    Q(moderator=request.user) &
    Q(
        playteammembership__user=request.user,
        playteammembership__user_accepted=True,
        playteammembership__playteam_accepted=True
      )
    ).order_by('name')

Please note the operator between two Q, which has been changed to AND. The result SQL is:

In [73]: p._get_sql_clause()
Out[73]: 
(['"playarena_playteam"."id"',
  '"playarena_playteam"."name"',
  '"playarena_playteam"."slug"',
  '"playarena_playteam"."description"',
  '"playarena_playteam"."city"',
  '"playarena_playteam"."province"',
  '"playarena_playteam"."moderator_id"',
  '"playarena_playteam"."active"',
  '"playarena_playteam"."players_wanted"'],
 ' FROM "playarena_playteam" INNER JOIN "playarena_playteammembership" AS "playarena_playteam__playteammembership" ON "playarena_playteam"."id" = "playarena_playteam__playteammembership"."playteam_id" WHERE (("playarena_playteam"."moderator_id" = %s AND "playarena_playteam__playteammembership"."user_id" = %s AND "playarena_playteam__playteammembership"."user_accepted" = %s AND "playarena_playteam__playteammembership"."playteam_accepted" = %s))',
 [1, 1, True, True])

All operators are AND now.

Looks like the operator applied to conditions inside single Q is a result of context where the Q has been used. This is definitely not intuitive behaviour, inconsistent with what we have in .filter() and .exclude() methods.

The final query which gives expected results is:

q = Playteam.objects.filter(
    Q(moderator=request.user) |
    (
        Q(playteammembership__user=request.user) &
        Q(playteammembership__user_accepted=True) &
        Q(playteammembership__playteam_accepted=True)
        )
      )
    ).order_by('name')

My suggestion is to make AND a default operator within single Q, in the same way as in .filter() or to forbid using multiple congitions in single Q at all.

Change History (5)

comment:1 Changed 7 years ago by gav

  • Keywords qsrf-cleanup added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 7 years ago by jacob

  • milestone set to 1.0

comment:3 Changed 7 years ago by mtredinnick

Based on the use of _get_sql_clause() in the example, I suspect this is using a checkout prior to the queryset-refactor merge. Could the original reporter please confirm that (at least confirm which version of code is being used).

I cannot repeat the problem with current trunk (line 307 of tests/regressiontests/queries/models.py is of the same and generates the correct query). If it's from prior to the queryset-refactor merge, it's already fixed. Leaving as is for now, but we need more information here and I suspect it's already fixed.

comment:4 Changed 7 years ago by mtredinnick

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

Just discovered where I'm doing a similar query in some more serious code and it also generates the right SQL (all keywords inside the Q() are AND-ed, always). So I'm closing this. Please reopen with version information and a short example (a small model plus a query, since the examples I have all work) if you still see the issue on trunk.

comment:5 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top