[dimitri-yatsenko]

Fixes #1650.

The bug

A.aggr(B & cond, ...) with the default exclude_nonmatching=False is documented to keep every row of A. It builds a LEFT JOIN, but the restriction on B landed in the outer WHERE, after the join. Rows of A with no match have NULL in B's columns, fail the condition, and drop out, so the result behaved like exclude_nonmatching=True:

Parent.aggr(Child & "color='red'", n="count(child_id)")
# before: [{'parent_id': 1, 'n': 1}]
# after:  [{'parent_id': 1, 'n': 1}, {'parent_id': 2, 'n': 0}, {'parent_id': 3, 'n': 0}]

The cause is in QueryExpression.join, so every left join had it:

• aggr with the default exclude_nonmatching=False
• extend: Child.extend(Parent & cond) dropped children whose parent failed the condition, instead of keeping them with NULLs.
• Union of operands that both have secondary attributes, which left-joins the second operand: Parent + (ParentInfo & cond) dropped parents.

The fix

In join(), when the join is a left join and the right operand is restricted, the right operand is wrapped in a subquery, so its restriction applies before the join:

... FROM parent LEFT JOIN (SELECT ... FROM child WHERE (color='red')) AS ... USING (parent_id) ...

• dj.Top counts as a restriction, since it is applied with &. It is stored in _top rather than in restriction, so the condition checks both.
• An unrestricted right operand is joined directly, with no subquery, exactly as before.
• Inner joins are unchanged. Their restrictions stay in WHERE, including exclude_nonmatching=True and dj.U(...).aggr(...).
• A restriction on the left operand still applies to the result.

Behavior change

Queries of the forms above now return the left rows they used to drop: zero counts, or NULL for other aggregates and for extend/union attributes. This matches the documented behavior. Code that relied on the filtering should pass exclude_nonmatching=True or use an inner join (*). The v2.3.7 release notes should say this.

Tests

tests/integration/test_aggr_left_join.py runs on both MySQL and PostgreSQL (38 tests):

• Restricted group: by a string, a mapping, a multi-key mapping, a list of mappings (OR), the join attribute itself, a table, a negation (by a table and by a string), dj.Top, and a restricted group followed by .proj(). Also covers a restriction on the grouping side, and a restriction on the computed count (& "n = 0", which goes through HAVING, or the subquery on PostgreSQL).
• extend and union with a restricted operand.
• Controls:
  • no subquery for an unrestricted group or an unrestricted extend operand
  • inner aggr, dj.U.aggr and * keep the restriction in WHERE
  • a restricted multi-table group, which aggr already wrapped in a subquery and which also gets NULL for max

Without the fix, 13 tests fail on each backend, which is every test that exercises the bug, and the 6 controls pass. With it, all 38 pass. The full suite passes locally: 1272 passed, 14 skipped.

Not changed here

An inner join also drops a dj.Top on either operand: Parent * (Child & dj.Top(1)) returns every matching row. That bug predates this PR and is not covered by #1650, so it is left for a separate fix.

[MilagrosMarin]

Thanks @dimitri-yatsenko, this is a clean, well-scoped fix.

✅ Wrapping the right operand in a subquery only when it's restricted (or carries a dj.Top) puts the restriction where it belongs and leaves unrestricted and inner joins unchanged.
✅ Checking other._top separately is needed: join() never carries _top onto the result, so without that check a Top-only group would be silently dropped. The dj.Top test covers it.
✅ The tests cover the restriction forms well, and the controls (inner aggr, dj.U.aggr, *, unrestricted extend) pin down what must not change.
✅ The docs already state that aggr keeps every row of the grouping table by default (reference/operators), so this brings the code in line with them. Agreed that it needs a release note.

Two small follow-ups, neither blocking:

• dj.Top on the left operand. From reading the code, join() keeps self.restriction but not self._top, and doesn't wrap self in a subquery when it has one. So (Parent & dj.Top(1)).aggr(Child, ...) and .extend(...) look like they drop the Top too. That's older than this PR, but it might be worth widening the "Not changed here" note (or the follow-up issue) from "inner join" to "either operand of any join".
• Possible typo (older code): in join(), need_subquery2 checks isinstance(self, Union). Should that be other?

Approving. Let me know if you'd like me to open an issue for the Top follow-up.

[dimitri-yatsenko]

Thanks @MilagrosMarin. On the two follow-ups:

• dj.Top on the left operand: fix: dj.Top is honored when a limited expression is used in another query #1657 handles it. With this PR and fix: dj.Top is honored when a limited expression is used in another query #1657 both applied, a Top on the left operand is honored for *, for aggr in both exclude_nonmatching modes, and for extend, on MySQL and PostgreSQL. fix: dj.Top is honored when a limited expression is used in another query #1657's tests cover those cases, so this PR stays scoped to the right operand.
• isinstance(self, Union) in need_subquery2: you're right, it should be other. The bug is older than this PR and is tracked in Joining with a union on the right raises AssertionError when the two sides share no attributes #1659: a union on the right of a cross join raises AssertionError, and under python -O it silently drops the second half of the union. It gets its own small PR with a test.

GitHub Actions has no run for this branch, because no workflows ran in this repository between Oct 31 and Nov 9. So before merging I merged it onto current master with the other approved PRs (#1651, #1652, #1653, #1654, #1657, #1643) and ran lint and the full suite locally. Everything passed: 1362 tests on MySQL 8.0 and 8.4, including the tests shared with PostgreSQL, and the suite at the minimum dependency versions on Python 3.10. Merging.