fix(postgres): correctly infer nullability for LEFT JOIN rewritten as RIGHT JOIN#4285
fix(postgres): correctly infer nullability for LEFT JOIN rewritten as RIGHT JOIN#4285luca992 wants to merge 2 commits into
Conversation
… RIGHT JOIN PostgreSQL's planner may execute `A LEFT JOIN B` as a hash join with `Join Type: Right` to put the smaller relation on the hash-build side. This is documented behavior of the planner: * [Postgres docs, 14.3 Controlling the Planner with Explicit JOIN Clauses] > "Most practical cases involving LEFT JOIN or RIGHT JOIN can be > rearranged to some extent." https://www.postgresql.org/docs/current/explicit-joins.html * [Postgres Pro, "Queries in PostgreSQL: 6. Hashing"] > "On the physical level, the planner determines which set is the > inner one and which is the outer one not by their positions in > the query, but by the relative join cost. ... So, the join type > switches from left to right in the plan." https://postgrespro.com/blog/pgsql/5969673 After the swap, the SQL right operand (the nullable side under LEFT JOIN semantics) appears as the plan's `Outer` child rather than the `Inner` child. The previous `visit_plan` only marked `Inner` children as nullable, which on a `Join Type: Right` plan: * incorrectly marked the SQL left operand (always preserved) as nullable — causing spurious `Option<T>` in macro output for NOT NULL columns; and * failed to mark the SQL right operand as nullable — masking real NULLs and panicking at decode time when no LEFT JOIN row matched. Thread the parent join type into `visit_plan` and decide which child is the NULL-fill side based on it: * `Left` → `Inner` child is nullable (no change) * `Right` → `Outer` child is nullable (new) * `Full` → both children are nullable (no change) Also recurse into all child plans (not only when the current node is `Left`/`Right`), so nested joins reached through non-join intermediates like `Hash` are walked. Closes transact-rs#3202.
977cbbb to
691420b
Compare
|
I observed an issue with In my case, the query plan on the unpopulated DB contains a root output of the shape The generated query plan for the populated DB is different and doesn't contain such an expression as one of its root outputs. |
|
@die-herdplatte hmm is there a way you can share for me to reproduce it? |
7f594c8 to
179277d
Compare
|
@die-herdplatte I made changes which I think should handle your issues if you want to check |
…in boundaries Walk each Left/Right join node's own outputs (not just its children), marking those not pass-through from the preserved-side child. Tolerate redundant outer parens in EXPLAIN deparse so root `((expr))` matches the join's `(expr)`. Add real-PG-derived regression tests.
179277d to
b9b36ae
Compare
PostgreSQL's planner may execute
A LEFT JOIN Bas a hash join withJoin Type: Rightto put the smaller relation on the hash-build side.This is documented behavior:
[Postgres docs, 14.3 Controlling the Planner with Explicit JOIN Clauses]
[Postgres Pro, "Queries in PostgreSQL: 6. Hashing"]
After the swap the SQL right operand (the nullable side under LEFT JOIN semantics) ends up as the plan's
Outerchild instead of theInnerchild. The oldvisit_planonly markedInnerchildren nullable, which on aJoin Type: Rightplan:Option<T>in macro output for NOT NULL columnsThe old
visit_planalso only inspected children of outer joins, never the join node's ownOutput. Computed expressions likeb.x || 'y'orCOALESCE(batx.X, NULL)materialize at the join node itself; the nullable child only carries raw column refs, so nullability inference silently dropped these columns. Postgres further deparses these expressions with an extra outer paren pair at root (((expr))) compared to the join node ((expr)), so exact-string matching missed them.Fix threads
in_nullablethroughvisit_planand picks the NULL-fill side from the current node's join type:Left->Innerchild is nullableRight->Outerchild is nullableFull-> both children are nullableIt also marks the join node's own outputs that aren't pass-throughs from the preserved-side child, catching computed expressions that don't appear on the nullable-side child. Output comparison normalizes redundant outer parens (Postgres-lexical tokenizer for
'…',"…",E'…',B'…'/X'…',U&'…'/U&"…",$tag$…$tag$) so root((expr))matches join(expr).It also recurses into all child plans, not only when the current node is
Left/Right, so nested joins reached through non-join intermediates likeHashare walked.Does your PR solve an issue?
fixes #3202
Is this a breaking change?
Behavior change yes. The old behavior was wrong inference, not a documented contract.
For queries with a LEFT JOIN that postgres rewrites as a Hash Right Join (driven by
pg_statisticcost estimates, fires on production sized data withplan_cache_mode = force_generic_planwhichsqlx-macros-coreitself sets), generated types change:Columns from the SQL left operand that are NOT NULL in their base table go from
Option<T>toT. So code handling these unneeded nulls would need to be dropped.Columns from the SQL right operand (the LEFT JOIN nullable side) go from
TtoOption<T>. Code treating these as non-null was already at risk ofunexpected null; try decoding as an Optionpanics at runtime whenever a row had no matching join partner. After this it stops compiling and the field needs to becomeOption<T>.Computed expressions on the nullable side of any outer join (
COALESCE(b.x, …),b.x || y, function calls,CASE, etc.) go fromTtoOption<T>. Same latent panic risk and same migration as the case above.The right-operand and computed-expression cases are the bigger ones. Code passed type checks before only because LEFT JOINs happened to always find a match in test data. After this fix the type system exposes the real nullability.