Skip to content

Commit 977cbbb

Browse files
committed
fix(postgres): correctly infer nullability for LEFT JOIN rewritten as 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 #3202.
1 parent 75bc048 commit 977cbbb

1 file changed

Lines changed: 292 additions & 11 deletions

File tree

sqlx-postgres/src/connection/describe.rs

Lines changed: 292 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -153,20 +153,42 @@ impl PgConnection {
153153
}) = explains.first()
154154
{
155155
nullables.resize(outputs.len(), None);
156-
visit_plan(plan, outputs, &mut nullables);
156+
visit_plan(plan, None, outputs, &mut nullables);
157157
}
158158

159159
Ok(nullables)
160160
}
161161
}
162162

163-
fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec<Option<bool>>) {
163+
fn visit_plan(
164+
plan: &Plan,
165+
parent_join_type: Option<&str>,
166+
outputs: &[String],
167+
nullables: &mut Vec<Option<bool>>,
168+
) {
164169
if let Some(plan_outputs) = &plan.output {
165-
// all outputs of a Full Join must be marked nullable
166-
// otherwise, all outputs of the inner half of an outer join must be marked nullable
167-
if plan.join_type.as_deref() == Some("Full")
168-
|| plan.parent_relation.as_deref() == Some("Inner")
169-
{
170+
// Determine whether THIS plan's outputs can be NULL due to its parent join.
171+
//
172+
// PostgreSQL may execute `A LEFT JOIN B` as a `Right` join when the planner
173+
// swaps the build/probe sides for hash join efficiency (e.g. when B is the
174+
// smaller of the two and is cheaper as the hash-build side). After that
175+
// swap, the operand that *was* the SQL right side (B, the nullable one)
176+
// appears as the "Outer" child of the plan node — not the "Inner" child.
177+
//
178+
// So the side that needs the nullable mark depends on `parent_join_type`:
179+
// * Left : Inner child is the nullable side (SQL right operand)
180+
// * Right : Outer child is the nullable side (SQL right operand, after swap)
181+
// * Full : both sides nullable
182+
let parent_nulls_this_side = match (parent_join_type, plan.parent_relation.as_deref()) {
183+
(Some("Full"), _) => true,
184+
(Some("Left"), Some("Inner")) => true,
185+
(Some("Right"), Some("Outer")) => true,
186+
_ => false,
187+
};
188+
189+
let self_is_full_join = plan.join_type.as_deref() == Some("Full");
190+
191+
if parent_nulls_this_side || self_is_full_join {
170192
for output in plan_outputs {
171193
if let Some(i) = outputs.iter().position(|o| o == output) {
172194
// N.B. this may produce false positives but those don't cause runtime errors
@@ -177,10 +199,11 @@ fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec<Option<bool>>
177199
}
178200

179201
if let Some(plans) = &plan.plans {
180-
if let Some("Left") | Some("Right") = plan.join_type.as_deref() {
181-
for plan in plans {
182-
visit_plan(plan, outputs, nullables);
183-
}
202+
// Recurse into all child plans so nested LEFT/RIGHT joins are reached even
203+
// if intermediate nodes are not joins themselves (e.g. a `Hash` node sitting
204+
// between two join nodes).
205+
for child in plans {
206+
visit_plan(child, plan.join_type.as_deref(), outputs, nullables);
184207
}
185208
}
186209
}
@@ -276,3 +299,261 @@ fn explain_parsing() {
276299
"unexpected parse from {utility_statement:?}: {utility_statement_parsed:?}"
277300
)
278301
}
302+
303+
#[cfg(test)]
304+
fn nullables_from_plan(plan_json: &str) -> Vec<Option<bool>> {
305+
let [Explain::Plan { plan }] = serde_json::from_str::<[Explain; 1]>(plan_json).unwrap() else {
306+
panic!("expected Explain::Plan, got something else");
307+
};
308+
let outputs = plan.output.clone().unwrap_or_default();
309+
let mut nullables = vec![None; outputs.len()];
310+
visit_plan(&plan, None, &outputs, &mut nullables);
311+
nullables
312+
}
313+
314+
// https://github.com/launchbadge/sqlx/issues/3202
315+
//
316+
// PostgreSQL rewrites `A LEFT JOIN B` as `B RIGHT JOIN A` to put the smaller
317+
// relation on the hash-build side. After the swap, the SQL right operand (the
318+
// nullable side) appears as the plan's `Outer` child, not the `Inner`.
319+
//
320+
// Plan is verbatim EXPLAIN (VERBOSE, FORMAT JSON) output of (the only SET
321+
// here, `plan_cache_mode`, is what `sqlx-macros-core` itself runs on each
322+
// connection used for describe):
323+
//
324+
// CREATE TABLE a (id uuid NOT NULL);
325+
// CREATE TABLE b (id uuid NOT NULL, name text NOT NULL);
326+
// INSERT INTO a SELECT gen_random_uuid() FROM generate_series(1, 1000);
327+
// INSERT INTO b SELECT gen_random_uuid(), 'b' FROM generate_series(1, 50000);
328+
// ANALYZE a; ANALYZE b;
329+
// SET plan_cache_mode = force_generic_plan;
330+
// PREPARE q(int) AS
331+
// SELECT a.id, b.name FROM a LEFT JOIN b ON a.id = b.id LIMIT $1;
332+
// EXPLAIN (VERBOSE, FORMAT JSON) EXECUTE q(NULL);
333+
#[test]
334+
fn nullable_inference_left_join_rewritten_as_right() {
335+
let plan = r#"
336+
[
337+
{
338+
"Plan": {
339+
"Node Type": "Limit",
340+
"Parallel Aware": false,
341+
"Async Capable": false,
342+
"Startup Cost": 28.50,
343+
"Total Cost": 130.15,
344+
"Plan Rows": 100,
345+
"Plan Width": 18,
346+
"Output": ["a.id", "b.name"],
347+
"Plans": [
348+
{
349+
"Node Type": "Hash Join",
350+
"Parent Relationship": "Outer",
351+
"Parallel Aware": false,
352+
"Async Capable": false,
353+
"Join Type": "Right",
354+
"Startup Cost": 28.50,
355+
"Total Cost": 1045.00,
356+
"Plan Rows": 1000,
357+
"Plan Width": 18,
358+
"Output": ["a.id", "b.name"],
359+
"Inner Unique": false,
360+
"Hash Cond": "(b.id = a.id)",
361+
"Plans": [
362+
{
363+
"Node Type": "Seq Scan",
364+
"Parent Relationship": "Outer",
365+
"Parallel Aware": false,
366+
"Async Capable": false,
367+
"Relation Name": "b",
368+
"Schema": "public",
369+
"Alias": "b",
370+
"Startup Cost": 0.00,
371+
"Total Cost": 819.00,
372+
"Plan Rows": 50000,
373+
"Plan Width": 18,
374+
"Output": ["b.id", "b.name"]
375+
},
376+
{
377+
"Node Type": "Hash",
378+
"Parent Relationship": "Inner",
379+
"Parallel Aware": false,
380+
"Async Capable": false,
381+
"Startup Cost": 16.00,
382+
"Total Cost": 16.00,
383+
"Plan Rows": 1000,
384+
"Plan Width": 16,
385+
"Output": ["a.id"],
386+
"Plans": [
387+
{
388+
"Node Type": "Seq Scan",
389+
"Parent Relationship": "Outer",
390+
"Parallel Aware": false,
391+
"Async Capable": false,
392+
"Relation Name": "a",
393+
"Schema": "public",
394+
"Alias": "a",
395+
"Startup Cost": 0.00,
396+
"Total Cost": 16.00,
397+
"Plan Rows": 1000,
398+
"Plan Width": 16,
399+
"Output": ["a.id"]
400+
}
401+
]
402+
}
403+
]
404+
}
405+
]
406+
}
407+
}
408+
]
409+
"#;
410+
// a.id (Inner branch, SQL left operand): preserved
411+
// b.name (Outer branch, SQL right operand): nullable
412+
assert_eq!(nullables_from_plan(plan), vec![None, Some(true)]);
413+
}
414+
415+
// Two nested LEFT JOINs both rewritten as Hash Right Join. Exercises
416+
// (a) recursion through a non-join `Hash` node sitting between two join
417+
// nodes, and (b) the rewrite being handled at every level.
418+
//
419+
// Plan is verbatim EXPLAIN (VERBOSE, FORMAT JSON) output of:
420+
//
421+
// CREATE TABLE c (id uuid NOT NULL, x text NOT NULL);
422+
// INSERT INTO c SELECT gen_random_uuid(), 'c' FROM generate_series(1, 50000);
423+
// ANALYZE c;
424+
// -- `a` and `b` are seeded as in the test above
425+
// SET plan_cache_mode = force_generic_plan;
426+
// PREPARE q(int) AS
427+
// SELECT a.id, b.name, c.x
428+
// FROM a
429+
// LEFT JOIN b ON a.id = b.id
430+
// LEFT JOIN c ON a.id = c.id
431+
// LIMIT $1;
432+
// EXPLAIN (VERBOSE, FORMAT JSON) EXECUTE q(NULL);
433+
#[test]
434+
fn nullable_inference_nested_left_joins_rewritten() {
435+
let plan = r#"
436+
[
437+
{
438+
"Plan": {
439+
"Node Type": "Limit",
440+
"Parallel Aware": false,
441+
"Async Capable": false,
442+
"Startup Cost": 1057.50,
443+
"Total Cost": 1159.15,
444+
"Plan Rows": 100,
445+
"Plan Width": 20,
446+
"Output": ["a.id", "b.name", "c.x"],
447+
"Plans": [
448+
{
449+
"Node Type": "Hash Join",
450+
"Parent Relationship": "Outer",
451+
"Parallel Aware": false,
452+
"Async Capable": false,
453+
"Join Type": "Right",
454+
"Startup Cost": 1057.50,
455+
"Total Cost": 2074.00,
456+
"Plan Rows": 1000,
457+
"Plan Width": 20,
458+
"Output": ["a.id", "b.name", "c.x"],
459+
"Inner Unique": false,
460+
"Hash Cond": "(c.id = a.id)",
461+
"Plans": [
462+
{
463+
"Node Type": "Seq Scan",
464+
"Parent Relationship": "Outer",
465+
"Parallel Aware": false,
466+
"Async Capable": false,
467+
"Relation Name": "c",
468+
"Schema": "public",
469+
"Alias": "c",
470+
"Startup Cost": 0.00,
471+
"Total Cost": 819.00,
472+
"Plan Rows": 50000,
473+
"Plan Width": 18,
474+
"Output": ["c.id", "c.x"]
475+
},
476+
{
477+
"Node Type": "Hash",
478+
"Parent Relationship": "Inner",
479+
"Parallel Aware": false,
480+
"Async Capable": false,
481+
"Startup Cost": 1045.00,
482+
"Total Cost": 1045.00,
483+
"Plan Rows": 1000,
484+
"Plan Width": 18,
485+
"Output": ["a.id", "b.name"],
486+
"Plans": [
487+
{
488+
"Node Type": "Hash Join",
489+
"Parent Relationship": "Outer",
490+
"Parallel Aware": false,
491+
"Async Capable": false,
492+
"Join Type": "Right",
493+
"Startup Cost": 28.50,
494+
"Total Cost": 1045.00,
495+
"Plan Rows": 1000,
496+
"Plan Width": 18,
497+
"Output": ["a.id", "b.name"],
498+
"Inner Unique": false,
499+
"Hash Cond": "(b.id = a.id)",
500+
"Plans": [
501+
{
502+
"Node Type": "Seq Scan",
503+
"Parent Relationship": "Outer",
504+
"Parallel Aware": false,
505+
"Async Capable": false,
506+
"Relation Name": "b",
507+
"Schema": "public",
508+
"Alias": "b",
509+
"Startup Cost": 0.00,
510+
"Total Cost": 819.00,
511+
"Plan Rows": 50000,
512+
"Plan Width": 18,
513+
"Output": ["b.id", "b.name"]
514+
},
515+
{
516+
"Node Type": "Hash",
517+
"Parent Relationship": "Inner",
518+
"Parallel Aware": false,
519+
"Async Capable": false,
520+
"Startup Cost": 16.00,
521+
"Total Cost": 16.00,
522+
"Plan Rows": 1000,
523+
"Plan Width": 16,
524+
"Output": ["a.id"],
525+
"Plans": [
526+
{
527+
"Node Type": "Seq Scan",
528+
"Parent Relationship": "Outer",
529+
"Parallel Aware": false,
530+
"Async Capable": false,
531+
"Relation Name": "a",
532+
"Schema": "public",
533+
"Alias": "a",
534+
"Startup Cost": 0.00,
535+
"Total Cost": 16.00,
536+
"Plan Rows": 1000,
537+
"Plan Width": 16,
538+
"Output": ["a.id"]
539+
}
540+
]
541+
}
542+
]
543+
}
544+
]
545+
}
546+
]
547+
}
548+
]
549+
}
550+
}
551+
]
552+
"#;
553+
// a.id (driving table) preserved through both LEFT JOINs.
554+
// b.name and c.x become NULL when their respective JOIN finds no match.
555+
assert_eq!(
556+
nullables_from_plan(plan),
557+
vec![None, Some(true), Some(true)]
558+
);
559+
}

0 commit comments

Comments
 (0)