Skip to content

Commit 1d6a98f

Browse files
author
Andrew Cheng
committed
Flatten q filter operands into a single WHERE where possible
Builds on the prior pk__in subquery rewrite. That change removed the set-operations, but still wrapped every operand of a NOT/AND/OR in its own pk__in subquery, so a q expression compiled to one subquery per operand and the SQL grew a nesting level for every AND/OR/NOT -- up to 8 stacked subqueries at the complexity cap. This is the "deep nesting" cost the reviewer asked about. Introduce a hybrid Q-composition. Each operand can now report an `as_q()`: - A leaf is Q-reducible only when it is a standard django-filter Filter (no custom .filter() override, no method=, no distinct=). It returns a plain `Q(field__lookup=value)` (negated for exclude filters). Opaque operands -- label filters, repository_version, href/prn/id resolvers, method filters, and ChoiceFilter (which overrides .filter() for null handling) -- return None. - NOT/AND/OR combine their children's Q objects with ~, &, | when all children are reducible. `evaluate()` then folds every reducible operand into a single flat Q and applies only the genuinely opaque operands as pk__in subqueries. An all-reducible expression -- however deeply nested -- collapses to one flat SELECT; mixed expressions keep one subquery per opaque operand. Opaque operands are always evaluated against the small base queryset, so they never re-embed an accumulated filter chain. Results are unchanged and the existing q filter functional tests (test_q_filter_valid / test_q_filter_invalid, 21 cases) pass without modification. Benchmarks (100k rows, steady-state, count(), best of 4), old set-ops vs. this change; SELECTs = number of SQL SELECT statements emitted: Repositories (all-reducible name/number fields) Expression set-ops this change ---------------------------------- -------------- -------------- name AND name 42 ms / 3 15 ms / 1 (A AND B) OR (C AND D) 79 ms / 7 20 ms / 1 (A AND (B OR C)) AND NOT D (cx 8) 144 ms / 8 19 ms / 1 name AND retain>=x AND retain<y 44 ms / 4 17 ms / 1 (num range) OR num=0 124 ms / 5 15 ms / 1 Tasks name AND NOT name 68 ms / 4 17 ms / 1 name AND (NOT state OR state) 119 ms / 6 90 ms / 5 ContentGuards (name) A AND B 56 ms / 3 19 ms / 1 (A AND B) OR C 155 ms / 5 20 ms / 1 Opaque-only expressions are unchanged: content repository_version `X AND NOT Y` over 200k units stays ~2.45 s (199k pair) with no regression, since both operands remain pk__in subqueries. The speedup grows with nesting depth because the old plan added a subquery per level while this one stays flat -- ~1.6x on a shallow OR up to ~8x at the complexity-8 limit. The remaining opaque cases (e.g. task `state`, a ChoiceFilter) could be reduced too in a follow-up. Refs #7831 Assisted by: Claude Opus 4.8
1 parent 9386abb commit 1d6a98f

2 files changed

Lines changed: 70 additions & 5 deletions

File tree

CHANGES/7831.bugfix

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1-
Optimized the ``q`` complex filter to compose ``NOT``/``AND``/``OR`` operands using ``pk__in``
2-
subqueries instead of the ``EXCEPT``/``INTERSECT``/``UNION`` set operations, avoiding full-table
3-
scans and disk-spilling sorts on large content sets.
1+
Optimized the `q` complex filter to compose `NOT`/`AND`/`OR` operands as flat `WHERE`
2+
conditions in a single SQL statement for standard field filters, falling back to `pk__in`
3+
subqueries only for opaque operands (label, `repository_version`, and href/prn/id resolvers).
4+
This replaces the previous `EXCEPT`/`INTERSECT`/`UNION` set operations, avoiding full-table
5+
scans and disk-spilling sorts, and keeps the query flat regardless of expression nesting depth.

pulpcore/filters.py

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,37 +206,100 @@ def __init__(self, filterset, tokens):
206206
self.value = form.cleaned_data[key]
207207
self.complexity = 1
208208

209+
def as_q(self):
210+
# A leaf reduces to a plain WHERE condition (Q) only when it is a standard
211+
# django-filter Filter: no custom .filter() override, no method, no .distinct().
212+
# Anything else (href/prn resolvers, repository_version, method filters, ...) is
213+
# opaque and must be applied via its own queryset transform in evaluate().
214+
f = self.filter
215+
if (
216+
type(f).filter is not filters.Filter.filter
217+
or getattr(f, "method", None) is not None
218+
or getattr(f, "distinct", False)
219+
):
220+
return None
221+
if self.value in EMPTY_VALUES:
222+
return models.Q()
223+
q = models.Q(**{f"{f.field_name}__{f.lookup_expr}": self.value})
224+
return ~q if f.exclude else q
225+
209226
def evaluate(self, qs):
227+
q = self.as_q()
228+
if q is not None:
229+
return qs.filter(q)
210230
return self.filter.filter(qs, self.value)
211231

212232
class _NotAction:
213233
def __init__(self, tokens):
214234
self.expr = tokens[0][0]
215235
self.complexity = self.expr.complexity + 1
216236

237+
def as_q(self):
238+
child = self.expr.as_q()
239+
return None if child is None else ~child
240+
217241
def evaluate(self, qs):
242+
q = self.as_q()
243+
if q is not None:
244+
return qs.filter(q)
245+
# Opaque operand: anti-join via a NOT IN subquery.
218246
return qs.exclude(pk__in=self.expr.evaluate(qs).values("pk"))
219247

220248
class _AndAction:
221249
def __init__(self, tokens):
222250
self.exprs = tokens[0]
223251
self.complexity = sum((expr.complexity for expr in self.exprs)) + 1
224252

253+
def as_q(self):
254+
children = [expr.as_q() for expr in self.exprs]
255+
if any(child is None for child in children):
256+
return None
257+
query = models.Q()
258+
for child in children:
259+
query &= child
260+
return query
261+
225262
def evaluate(self, qs):
263+
# Fold every Q-reducible operand into a single flat WHERE, and apply only the
264+
# genuinely opaque operands as pk__in semi-join subqueries.
226265
result = qs
266+
combined = models.Q()
267+
has_q = False
227268
for expr in self.exprs:
228-
result = result.filter(pk__in=expr.evaluate(qs).values("pk"))
269+
child = expr.as_q()
270+
if child is not None:
271+
combined &= child
272+
has_q = True
273+
else:
274+
result = result.filter(pk__in=expr.evaluate(qs).values("pk"))
275+
if has_q:
276+
result = result.filter(combined)
229277
return result
230278

231279
class _OrAction:
232280
def __init__(self, tokens):
233281
self.exprs = tokens[0]
234282
self.complexity = sum((expr.complexity for expr in self.exprs)) + 1
235283

284+
def as_q(self):
285+
children = [expr.as_q() for expr in self.exprs]
286+
if any(child is None for child in children):
287+
return None
288+
query = children[0]
289+
for child in children[1:]:
290+
query |= child
291+
return query
292+
236293
def evaluate(self, qs):
294+
# Reducible operands stay as flat OR'd conditions; opaque operands contribute a
295+
# pk__in subquery. Both are combined in a single WHERE clause.
237296
query = models.Q()
238297
for expr in self.exprs:
239-
query |= models.Q(pk__in=expr.evaluate(qs).values("pk"))
298+
child = expr.as_q()
299+
if child is not None:
300+
query |= child
301+
else:
302+
query |= models.Q(pk__in=expr.evaluate(qs).values("pk"))
240303
return qs.filter(query)
241304

242305
def __init__(self, *args, **kwargs):

0 commit comments

Comments
 (0)