Skip to content

Commit 691420b

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 691420b

1 file changed

Lines changed: 290 additions & 11 deletions

File tree

sqlx-postgres/src/connection/describe.rs

Lines changed: 290 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -153,20 +153,40 @@ 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 = matches!(
183+
(parent_join_type, plan.parent_relation.as_deref()),
184+
(Some("Full"), _) | (Some("Left"), Some("Inner")) | (Some("Right"), Some("Outer"))
185+
);
186+
187+
let self_is_full_join = plan.join_type.as_deref() == Some("Full");
188+
189+
if parent_nulls_this_side || self_is_full_join {
170190
for output in plan_outputs {
171191
if let Some(i) = outputs.iter().position(|o| o == output) {
172192
// N.B. this may produce false positives but those don't cause runtime errors
@@ -177,10 +197,11 @@ fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec<Option<bool>>
177197
}
178198

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

0 commit comments

Comments
 (0)