Skip to content

Commit d1b307a

Browse files
committed
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.
1 parent 8873824 commit d1b307a

1 file changed

Lines changed: 28 additions & 13 deletions

File tree

sqlx-postgres/src/connection/describe.rs

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ impl PgConnection {
133133

134134
/// Infer nullability for columns of this statement using EXPLAIN VERBOSE.
135135
///
136-
/// This currently only marks columns that are on the inner half of an outer join
136+
/// This currently only marks columns that an outer join can set to `NULL`
137137
/// and returns `None` for all others.
138138
async fn nullables_from_explain(
139139
&mut self,
@@ -177,20 +177,28 @@ impl PgConnection {
177177
}) = explains.first()
178178
{
179179
nullables.resize(outputs.len(), None);
180-
visit_plan(plan, outputs, &mut nullables);
180+
visit_plan(plan, outputs, &mut nullables, false);
181181
}
182182

183183
Ok(nullables)
184184
}
185185
}
186186

187-
fn visit_plan(plan: &Plan, outputs: &[String], nullables: &mut Vec<Option<bool>>) {
188-
if let Some(plan_outputs) = &plan.output {
189-
// all outputs of a Full Join must be marked nullable
190-
// otherwise, all outputs of the inner half of an outer join must be marked nullable
191-
if plan.join_type.as_deref() == Some("Full")
192-
|| plan.parent_relation.as_deref() == Some("Inner")
193-
{
187+
/// Mark every output of this plan that an outer join can set to `NULL`.
188+
///
189+
/// `null_extended` is true when this plan is the null-extended input of an outer join above it.
190+
/// `visit_plan` visits every child, because a join can sit below any node, such as `Limit`.
191+
fn visit_plan(
192+
plan: &Plan,
193+
outputs: &[String],
194+
nullables: &mut Vec<Option<bool>>,
195+
null_extended: bool,
196+
) {
197+
// all outputs of a Full Join must be marked nullable
198+
let null_extended = null_extended || plan.join_type.as_deref() == Some("Full");
199+
200+
if null_extended {
201+
if let Some(plan_outputs) = &plan.output {
194202
for output in plan_outputs {
195203
if let Some(i) = outputs.iter().position(|o| o == output) {
196204
// 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<Option<bool>>
201209
}
202210

203211
if let Some(plans) = &plan.plans {
204-
if let Some("Left") | Some("Right") = plan.join_type.as_deref() {
205-
for plan in plans {
206-
visit_plan(plan, outputs, nullables);
207-
}
212+
for child in plans {
213+
let child_null_extended = match plan.join_type.as_deref() {
214+
// PostgreSQL defines `JOIN_RIGHT` as the mirror of `JOIN_LEFT`, so the
215+
// null-extended input is the Inner child of a Left join and the Outer
216+
// child of a Right join. See <https://github.com/launchbadge/sqlx/issues/367>.
217+
Some("Left") => child.parent_relation.as_deref() == Some("Inner"),
218+
Some("Right") => child.parent_relation.as_deref() == Some("Outer"),
219+
_ => false,
220+
};
221+
222+
visit_plan(child, outputs, nullables, child_null_extended);
208223
}
209224
}
210225
}

0 commit comments

Comments
 (0)