From 887382437993ec94e2552a64a3a609d34f5f0574 Mon Sep 17 00:00:00 2001 From: heios <40836953+heios@users.noreply.github.com> Date: Fri, 14 Aug 2026 05:11:19 +0100 Subject: [PATCH 1/2] test(postgres): cover outer joins the planner commutes or nests The planner commutes a plain `LEFT JOIN` into a `Right` join node, and it puts non-join nodes such as `Limit` above joins. Nullability inference marks the wrong side in the first shape, and marks nothing in the second shape. --- tests/postgres/postgres.rs | 62 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/postgres/postgres.rs b/tests/postgres/postgres.rs index 126771565a..1d03e383e1 100644 --- a/tests/postgres/postgres.rs +++ b/tests/postgres/postgres.rs @@ -1032,6 +1032,68 @@ from (values (null)) vals(val) assert_eq!(describe.nullable(0), Some(true)); assert_eq!(describe.nullable(1), Some(true)); + // a left join the planner commutes into a `Join Type: Right` node + // language=PostgreSQL + let describe = conn + .describe( + "select tweet.text, tweet_reply.text + from tweet + left join tweet_reply on tweet_reply.tweet_id = tweet.id" + .into_sql_str(), + ) + .await?; + + // tweet.text is on the preserved half, so it must stay NOT NULL + assert_eq!(describe.nullable(0), Some(false)); + assert_eq!(describe.nullable(1), Some(true)); + + // two chained left joins, which nest two `Right` nodes + // language=PostgreSQL + let describe = conn + .describe( + "select tweet.text, reply1.text, reply2.text + from tweet + left join tweet_reply reply1 on reply1.tweet_id = tweet.id + left join tweet_reply reply2 on reply2.tweet_id = tweet.id" + .into_sql_str(), + ) + .await?; + + assert_eq!(describe.nullable(0), Some(false)); + assert_eq!(describe.nullable(1), Some(true)); + assert_eq!(describe.nullable(2), Some(true)); + + // a join below a node that is not a join + // language=PostgreSQL + let describe = conn + .describe( + "select tweet.text, tweet_reply.text + from tweet + left join tweet_reply on tweet_reply.tweet_id = tweet.id + limit 5" + .into_sql_str(), + ) + .await?; + + assert_eq!(describe.nullable(0), Some(false)); + assert_eq!(describe.nullable(1), Some(true)); + + // the same query with `order by`. The planner gives it the opposite join type from the + // `limit` case, so the two cases together cover a `Left` node and a `Right` node. + // language=PostgreSQL + let describe = conn + .describe( + "select tweet.text, tweet_reply.text + from tweet + left join tweet_reply on tweet_reply.tweet_id = tweet.id + order by tweet.id" + .into_sql_str(), + ) + .await?; + + assert_eq!(describe.nullable(0), Some(false)); + assert_eq!(describe.nullable(1), Some(true)); + Ok(()) } From d1b307a2a93d5987e8000fa4ada97e42e70b7cfb Mon Sep 17 00:00:00 2001 From: heios <40836953+heios@users.noreply.github.com> Date: Fri, 14 Aug 2026 05:11:19 +0100 Subject: [PATCH 2/2] fix(postgres): mark the null-extended side of every outer join PostgreSQL defines `JOIN_RIGHT` as "pairs + unmatched RHS tuples", so the Outer child of a `Right` join node is the null-extended side, not the Inner child. `visit_plan` now passes a `null_extended` flag to the null-extended input, and visits every child, so a node such as `Limit` no longer ends the walk. --- sqlx-postgres/src/connection/describe.rs | 41 ++++++++++++++++-------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/sqlx-postgres/src/connection/describe.rs b/sqlx-postgres/src/connection/describe.rs index ee9918909d..2bbe9d76bb 100644 --- a/sqlx-postgres/src/connection/describe.rs +++ b/sqlx-postgres/src/connection/describe.rs @@ -133,7 +133,7 @@ impl PgConnection { /// Infer nullability for columns of this statement using EXPLAIN VERBOSE. /// - /// This currently only marks columns that are on the inner half of an outer join + /// This currently only marks columns that an outer join can set to `NULL` /// and returns `None` for all others. async fn nullables_from_explain( &mut self, @@ -177,20 +177,28 @@ impl PgConnection { }) = explains.first() { nullables.resize(outputs.len(), None); - visit_plan(plan, outputs, &mut nullables); + visit_plan(plan, outputs, &mut nullables, false); } Ok(nullables) } } -fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec>) { - if let Some(plan_outputs) = &plan.output { - // all outputs of a Full Join must be marked nullable - // otherwise, all outputs of the inner half of an outer join must be marked nullable - if plan.join_type.as_deref() == Some("Full") - || plan.parent_relation.as_deref() == Some("Inner") - { +/// Mark every output of this plan that an outer join can set to `NULL`. +/// +/// `null_extended` is true when this plan is the null-extended input of an outer join above it. +/// `visit_plan` visits every child, because a join can sit below any node, such as `Limit`. +fn visit_plan( + plan: &Plan, + outputs: &[String], + nullables: &mut Vec>, + null_extended: bool, +) { + // all outputs of a Full Join must be marked nullable + let null_extended = null_extended || plan.join_type.as_deref() == Some("Full"); + + if null_extended { + if let Some(plan_outputs) = &plan.output { for output in plan_outputs { if let Some(i) = outputs.iter().position(|o| o == output) { // N.B. this may produce false positives but those don't cause runtime errors @@ -201,10 +209,17 @@ fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec> } if let Some(plans) = &plan.plans { - if let Some("Left") | Some("Right") = plan.join_type.as_deref() { - for plan in plans { - visit_plan(plan, outputs, nullables); - } + for child in plans { + let child_null_extended = match plan.join_type.as_deref() { + // PostgreSQL defines `JOIN_RIGHT` as the mirror of `JOIN_LEFT`, so the + // null-extended input is the Inner child of a Left join and the Outer + // child of a Right join. See . + Some("Left") => child.parent_relation.as_deref() == Some("Inner"), + Some("Right") => child.parent_relation.as_deref() == Some("Outer"), + _ => false, + }; + + visit_plan(child, outputs, nullables, child_null_extended); } } }