From cf9129e2d4940c2588e76feac83a2dc331d13622 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 10 Oct 2026 13:10:07 +0100 Subject: [PATCH 1/2] fix(joins): single-equality RIGHT JOIN layout and operand routing (P63, P64) P63: a single plain-column equality RIGHT JOIN ran the LEFT hash builder with the tables swapped and kept the swapped layout, so a.* and b.* came back exchanged on every row. New `hash_join_right` indexes the FROM table and probes with each joined row; its result table comes from `right_join_result_table`, factored out of `nested_loop_join_right_multi` so the two RIGHT builders share one layout. P64, found by P63's test: the simple-column resolver took the written-left name in the FROM table and dropped the qualifier, so `b.id = a.partner_id` ran as `a.id = b.partner_id` whenever both tables hold both columns (any self-join), on INNER, LEFT and RIGHT. Operands are now routed by alias with P7's `operand_uses_right` and swapped, operator reversed, before their columns resolve. Unused column-name parameters on the inner/left hash builders removed. Parity 197 -> 202 AGREE / 233; the five cases that moved are exactly the two P63 and three new P64 cases. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/data/hash_join.rs | 218 +++++++++++------- tests/comparison/corpus/04_joins.toml | 24 +- .../comparison/corpus/10_aggregate_nulls.toml | 2 +- tests/join_operand_order_tests.rs | 94 ++++++++ 4 files changed, 257 insertions(+), 81 deletions(-) diff --git a/src/data/hash_join.rs b/src/data/hash_join.rs index 1ebeacd3..83e5653a 100644 --- a/src/data/hash_join.rs +++ b/src/data/hash_join.rs @@ -167,22 +167,41 @@ impl HashJoinExecutor { break; } + // Route the operands by alias, not by where they are written (P7, + // P64): `b.id = a.partner_id` is `a.partner_id = b.id`. Column + // lookup drops the qualifier, so without this a self-join resolves + // each name in the wrong table and pairs the wrong rows. + let (mut from_col_name, mut joined_col_name) = + (left_col_name.unwrap(), right_col_name.unwrap()); + let mut operator = single_condition.operator.clone(); + let written_reversed = self.operand_uses_right( + &single_condition.left_expr, + &join_clause.alias, + true, + false, + ) && !self.operand_uses_right( + &single_condition.right_expr, + &join_clause.alias, + true, + true, + ); + if written_reversed { + std::mem::swap(&mut from_col_name, &mut joined_col_name); + operator = self.reverse_operator(&operator); + } + let (left_col_idx, right_col_idx) = self.resolve_join_columns( &left_table, &right_table, - &left_col_name.unwrap(), - &right_col_name.unwrap(), + &from_col_name, + &joined_col_name, )?; - if single_condition.operator != JoinOperator::Equal { + if operator != JoinOperator::Equal { all_equal = false; } - condition_indices.push(( - left_col_idx, - right_col_idx, - single_condition.operator.clone(), - )); + condition_indices.push((left_col_idx, right_col_idx, operator)); } // Choose join algorithm based on operators - use hash join only if: @@ -196,21 +215,11 @@ impl HashJoinExecutor { if use_hash_join && condition_indices.len() == 1 { // Single equality condition with simple columns - use optimized hash join let (left_col_idx, right_col_idx, _) = condition_indices[0]; - let left_col_name = Self::extract_simple_column_name( - &join_clause.condition.conditions[0].left_expr, - ) - .expect("left_expr should be a simple column in hash join path"); - let right_col_name = Self::extract_simple_column_name( - &join_clause.condition.conditions[0].right_expr, - ) - .expect("right_expr should be a simple column in hash join path"); self.hash_join_inner( left_table, right_table, left_col_idx, right_col_idx, - &left_col_name, - &right_col_name, &join_clause.alias, ) } else { @@ -228,21 +237,11 @@ impl HashJoinExecutor { if use_hash_join && condition_indices.len() == 1 { // Single equality condition with simple columns - use optimized hash join let (left_col_idx, right_col_idx, _) = condition_indices[0]; - let left_col_name = Self::extract_simple_column_name( - &join_clause.condition.conditions[0].left_expr, - ) - .expect("left_expr should be a simple column in hash join path"); - let right_col_name = Self::extract_simple_column_name( - &join_clause.condition.conditions[0].right_expr, - ) - .expect("right_expr should be a simple column in hash join path"); self.hash_join_left( left_table, right_table, left_col_idx, right_col_idx, - &left_col_name, - &right_col_name, &join_clause.alias, ) } else { @@ -257,30 +256,16 @@ impl HashJoinExecutor { } } JoinType::Right => { - // Swap condition indices for right join - let swapped_indices: Vec<(usize, usize, JoinOperator)> = condition_indices - .into_iter() - .map(|(l, r, op)| (r, l, self.reverse_operator(&op))) - .collect(); - - if use_hash_join && swapped_indices.len() == 1 { - // Right join is just a left join with tables swapped - let (right_col_idx, left_col_idx, _) = swapped_indices[0]; - let left_col_name = Self::extract_simple_column_name( - &join_clause.condition.conditions[0].left_expr, - ) - .expect("left_expr should be a simple column in hash join path"); - let right_col_name = Self::extract_simple_column_name( - &join_clause.condition.conditions[0].right_expr, - ) - .expect("right_expr should be a simple column in hash join path"); - self.hash_join_left( - right_table, + if use_hash_join && condition_indices.len() == 1 { + // Single equality condition with simple columns. Not a LEFT + // join with the tables swapped: that swaps the result + // layout too (P63, P8's fault on the hash path). + let (from_col_idx, joined_col_idx, _) = condition_indices[0]; + self.hash_join_right( left_table, - right_col_idx, - left_col_idx, - &right_col_name, - &left_col_name, + right_table, + from_col_idx, + joined_col_idx, &join_clause.alias, ) } else { @@ -473,8 +458,6 @@ impl HashJoinExecutor { right_table: Arc, left_col_idx: usize, right_col_idx: usize, - _left_col_name: &str, - _right_col_name: &str, join_alias: &Option, ) -> Result { let start = std::time::Instant::now(); @@ -652,8 +635,6 @@ impl HashJoinExecutor { right_table: Arc, left_col_idx: usize, right_col_idx: usize, - _left_col_name: &str, - _right_col_name: &str, join_alias: &Option, ) -> Result { let start = std::time::Instant::now(); @@ -1206,41 +1187,81 @@ impl HashJoinExecutor { Ok(result) } - /// Nested loop join for RIGHT JOIN with multiple conditions (P8). - /// - /// A RIGHT JOIN keeps every row of the *joined* table (`b`), NULL-filling the - /// FROM table (`a`) where nothing matches. It is tempting to model this as a - /// LEFT join with the two tables swapped, but that swaps the *result* layout - /// too: columns come out `[joined, FROM]` and the join alias lands on the - /// swapped-in FROM table, so `a.*`/`b.*` labels (and the NULL side) invert. - /// - /// This builder instead keeps the physical layout stable and inverts only the - /// iteration/NULL-fill direction: - /// - result columns are `[FROM (a.*), joined (b.*)]`, exactly like INNER/LEFT, - /// so the FROM table keeps its qualified names and only the joined table - /// carries the join alias on a name collision; - /// - the outer loop is over the joined table so every `b` row is emitted in - /// `b` order, with the FROM columns NULLed when no `a` row matches. + /// Hash join for a single-equality RIGHT OUTER JOIN (P63). /// - /// Operand routing (P7) is preserved: `a.price < b.price` evaluates against the - /// tables named by the aliases regardless of which side is written first. - fn nested_loop_join_right_multi( + /// Indexes the FROM table and probes it with every joined-table row, so + /// each joined row is kept, in order, with the FROM columns NULLed when + /// nothing matches. The result layout is `nested_loop_join_right_multi`'s. + fn hash_join_right( &self, from_table: Arc, joined_table: Arc, - conditions: &[crate::sql::parser::ast::SingleJoinCondition], + from_col_idx: usize, + joined_col_idx: usize, join_alias: &Option, ) -> Result { let start = std::time::Instant::now(); + // Coerce string keys only when the join columns differ in type. + let coerce = join_key_coercion(&from_table, from_col_idx, &joined_table, joined_col_idx); + + // A NULL key equals nothing, not even another NULL (P52), so it is + // never indexed: a NULL probe then finds no match and is kept unmatched. + let mut hash_index: HashMap> = HashMap::new(); + for (row_idx, row) in from_table.rows.iter().enumerate() { + if matches!(row.values[from_col_idx], DataValue::Null) { + continue; + } + let key = canonical_join_key(&row.values[from_col_idx], coerce); + hash_index.entry(key).or_default().push(row_idx); + } + + let mut result = Self::right_join_result_table(&from_table, &joined_table, join_alias); + + let mut match_count = 0; + let mut null_count = 0; + + for joined_row in &joined_table.rows { + let joined_key = canonical_join_key(&joined_row.values[joined_col_idx], coerce); + + if let Some(matching_indices) = hash_index.get(&joined_key) { + for &from_idx in matching_indices { + let mut row = DataRow { values: Vec::new() }; + row.values + .extend_from_slice(&from_table.rows[from_idx].values); + row.values.extend_from_slice(&joined_row.values); + result.add_row(row).map_err(|e| anyhow!(e))?; + match_count += 1; + } + } else { + let mut row = DataRow { values: Vec::new() }; + row.values + .resize(from_table.column_count(), DataValue::Null); + row.values.extend_from_slice(&joined_row.values); + result.add_row(row).map_err(|e| anyhow!(e))?; + null_count += 1; + } + } + info!( - "Executing nested loop RIGHT JOIN with {} conditions: {} x {} rows", - conditions.len(), - from_table.row_count(), - joined_table.row_count() + "RIGHT JOIN complete: {} matches, {} nulls in {:?}", + match_count, + null_count, + start.elapsed() ); - // Create result table with columns in [FROM, joined] order. + Ok(result) + } + + /// The empty result table of a RIGHT JOIN, columns in `[FROM (a.*), joined + /// (b.*)]` order — the same layout as INNER and LEFT. The FROM side is + /// nullable (unmatched joined rows NULL-fill it), and only the joined table + /// takes the join alias on a name collision (P8, P63). + fn right_join_result_table( + from_table: &DataTable, + joined_table: &DataTable, + join_alias: &Option, + ) -> DataTable { let mut result = DataTable::new("joined"); // Add columns from the FROM table (a.*), unchanged — these are NULLable @@ -1301,6 +1322,45 @@ impl HashJoinExecutor { } } + result + } + + /// Nested loop join for RIGHT JOIN with multiple conditions (P8). + /// + /// A RIGHT JOIN keeps every row of the *joined* table (`b`), NULL-filling the + /// FROM table (`a`) where nothing matches. It is tempting to model this as a + /// LEFT join with the two tables swapped, but that swaps the *result* layout + /// too: columns come out `[joined, FROM]` and the join alias lands on the + /// swapped-in FROM table, so `a.*`/`b.*` labels (and the NULL side) invert. + /// + /// This builder instead keeps the physical layout stable and inverts only the + /// iteration/NULL-fill direction: + /// - result columns are `[FROM (a.*), joined (b.*)]`, exactly like INNER/LEFT, + /// so the FROM table keeps its qualified names and only the joined table + /// carries the join alias on a name collision; + /// - the outer loop is over the joined table so every `b` row is emitted in + /// `b` order, with the FROM columns NULLed when no `a` row matches. + /// + /// Operand routing (P7) is preserved: `a.price < b.price` evaluates against the + /// tables named by the aliases regardless of which side is written first. + fn nested_loop_join_right_multi( + &self, + from_table: Arc, + joined_table: Arc, + conditions: &[crate::sql::parser::ast::SingleJoinCondition], + join_alias: &Option, + ) -> Result { + let start = std::time::Instant::now(); + + info!( + "Executing nested loop RIGHT JOIN with {} conditions: {} x {} rows", + conditions.len(), + from_table.row_count(), + joined_table.row_count() + ); + + let mut result = Self::right_join_result_table(&from_table, &joined_table, join_alias); + // Create evaluators for both sides. The join-alias (joined) table is the // "right" evaluator, so alias-qualified operands route correctly (P7). let mut from_evaluator = diff --git a/tests/comparison/corpus/04_joins.toml b/tests/comparison/corpus/04_joins.toml index 7c439566..ba7e3e50 100644 --- a/tests/comparison/corpus/04_joins.toml +++ b/tests/comparison/corpus/04_joins.toml @@ -103,10 +103,32 @@ expect = "GAP" id = "right_join_single_condition" data = "null_edges.csv" sql = "SELECT a.id AS aid, b.id AS bid, b.team AS bteam FROM null_edges a RIGHT JOIN null_edges b ON a.partner_id = b.id ORDER BY b.id" -expect = "DIFFER" # P63, found 2026-10-03 in R13 slice 6. A single-equality RIGHT join takes the # hash path as a LEFT join with the tables swapped, and keeps the swapped # column order and labels - P8's fault, on the path P8's scope note called # safe. Every row is wrong, matched or not: `b.id = 1` comes back paired with # a.id 3 (b's own partner) instead of 2, and the unmatched b rows come back # with b NULL. There was no single-condition RIGHT JOIN case before this one. +# FIXED 2026-10-10: `hash_join_right`, sharing P8's column layout. + +# P64, found 2026-10-10 while fixing P63: the single-equality hash path routed +# its two operands by where they are written, not by alias (P7's fault, fixed +# only on the nested loop). `b.id = a.partner_id` was run as +# `a.id = b.partner_id` - silent, and only visible where both tables have both +# columns, as in a self-join. The P7 cases above all have two conditions, so +# they take the nested loop. FIXED 2026-10-10, same PR as P63: operands are +# routed by alias before their columns resolve. +[[case]] +id = "inner_join_single_condition_joined_side_first" +data = "null_edges.csv" +sql = "SELECT a.id AS aid, b.id AS bid FROM null_edges a JOIN null_edges b ON b.id = a.partner_id ORDER BY a.id" + +[[case]] +id = "left_join_single_condition_joined_side_first" +data = "null_edges.csv" +sql = "SELECT a.id AS aid, b.id AS bid FROM null_edges a LEFT JOIN null_edges b ON b.id = a.partner_id ORDER BY a.id" + +[[case]] +id = "right_join_single_condition_joined_side_first" +data = "null_edges.csv" +sql = "SELECT a.id AS aid, b.id AS bid FROM null_edges a RIGHT JOIN null_edges b ON b.id = a.partner_id ORDER BY b.id" diff --git a/tests/comparison/corpus/10_aggregate_nulls.toml b/tests/comparison/corpus/10_aggregate_nulls.toml index 793dba42..2684b057 100644 --- a/tests/comparison/corpus/10_aggregate_nulls.toml +++ b/tests/comparison/corpus/10_aggregate_nulls.toml @@ -136,13 +136,13 @@ sql = "SELECT a.id AS aid, b.id AS bid FROM null_edges a LEFT JOIN null_edges b id = "right_join_on_null_key_both_sides" data = "null_edges.csv" sql = "SELECT a.id AS aid, b.id AS bid FROM null_edges a RIGHT JOIN null_edges b ON a.label = b.label ORDER BY b.id, a.id" -expect = "DIFFER" # P52, hash path via the swapped LEFT join. DuckDB 12 rows, we returned 32. # P52's half FIXED 2026-10-03 (R13 slice 6): 12 rows now. Still DIFFER because # of P63, which this case uncovered - the single-condition RIGHT join labels # the swapped tables backwards, so the 5 unmatched rows come back with `a` # filled and `b` NULL. The old 5 x 5 NULL pairing had hidden it: a symmetric # self-join where every row matches looks the same either way round. +# AGREE since P63's fix, 2026-10-10. [[case]] id = "join_on_null_keys_two_conditions" diff --git a/tests/join_operand_order_tests.rs b/tests/join_operand_order_tests.rs index 72a5a6c6..2910317f 100644 --- a/tests/join_operand_order_tests.rs +++ b/tests/join_operand_order_tests.rs @@ -241,3 +241,97 @@ fn right_join_multi_condition_labels_correct_side() { } } } + +/// An `edges` table whose join is *asymmetric*: `a.partner_id = b.id` pairs +/// different rows than `a.id = b.partner_id`, so swapping the two sides of a +/// RIGHT JOIN changes the answer. (P8's test above self-joins on `symbol`, +/// which is symmetric — the reason P63 hid.) +fn edges_table() -> DataTable { + let mut table = DataTable::new("edges"); + table.add_column(DataColumn::new("id").with_type(DataType::Integer)); + table.add_column(DataColumn::new("partner_id").with_type(DataType::Integer)); + for (id, partner) in [(1, Some(2)), (2, Some(3)), (3, None), (4, Some(1))] { + let _ = table.add_row(DataRow { + values: vec![ + DataValue::Integer(id), + partner.map_or(DataValue::Null, DataValue::Integer), + ], + }); + } + table +} + +/// Run a query over `edges` and return its sorted `(aid, bid)` rows; `bid` is +/// the RIGHT JOIN's kept side, so it is never NULL. +fn aid_bid_rows_right(sql: &str) -> Vec<(Option, i64)> { + let context = &mut ExecutionContext::new(Arc::new(edges_table())); + let executor = StatementExecutor::new(); + let mut parser = Parser::new(sql); + let stmt = parser + .parse() + .unwrap_or_else(|e| panic!("parse failed for `{sql}`: {e}")); + let result = executor + .execute(stmt, context) + .unwrap_or_else(|e| panic!("exec failed for `{sql}`: {e}")); + + let view = &result.dataview; + let src = view.source(); + let aid_idx = src.get_column_index("aid").expect("aid column"); + let bid_idx = src.get_column_index("bid").expect("bid column"); + + let as_int = |v: &DataValue| -> Option { + match v { + DataValue::Integer(i) => Some(*i), + DataValue::Null => None, + other => panic!("unexpected id value: {other:?}"), + } + }; + + let mut rows: Vec<(Option, i64)> = (0..view.row_count()) + .map(|r| { + let aid = as_int(&src.get_value(r, aid_idx).expect("aid value")); + let bid = as_int(&src.get_value(r, bid_idx).expect("bid value")) + .expect("bid is never NULL on the RIGHT outer side"); + (aid, bid) + }) + .collect(); + rows.sort(); + rows +} + +#[test] +fn right_join_single_condition_labels_correct_side() { + // P63: a single plain-column equality RIGHT JOIN takes the hash path, which + // used to run the LEFT builder with the tables swapped and keep the swapped + // layout — every row came back with `a` and `b` exchanged. + let expected = vec![ + (None, 4), // nothing has partner 4 + (Some(1), 2), // a=1 partners b=2 + (Some(2), 3), // a=2 partners b=3 + (Some(4), 1), // a=4 partners b=1 + ]; + let mut expected_sorted = expected.clone(); + expected_sorted.sort(); + + for sql in [ + "SELECT a.id AS aid, b.id AS bid FROM edges a RIGHT JOIN edges b ON a.partner_id = b.id", + // Operands written joined-side first — the same predicate. + "SELECT a.id AS aid, b.id AS bid FROM edges a RIGHT JOIN edges b ON b.id = a.partner_id", + ] { + assert_eq!(aid_bid_rows_right(sql), expected_sorted, "`{sql}`"); + } +} + +#[test] +fn right_join_single_condition_matches_multi_condition_path() { + // The hash path (one equality) and the nested loop (P8's builder, reached + // here by a tautological second condition) must give the same RIGHT JOIN. + let hash = aid_bid_rows_right( + "SELECT a.id AS aid, b.id AS bid FROM edges a RIGHT JOIN edges b ON a.partner_id = b.id", + ); + let nested = aid_bid_rows_right( + "SELECT a.id AS aid, b.id AS bid FROM edges a RIGHT JOIN edges b \ + ON a.partner_id = b.id AND b.id >= 0", + ); + assert_eq!(hash, nested); +} From b3699ee319605664817b04085f5e7849bf723746 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Sat, 10 Oct 2026 13:10:07 +0100 Subject: [PATCH 2/2] docs(parity): close P63, file and close P64 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/SQL_PARITY.md | 64 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/docs/SQL_PARITY.md b/docs/SQL_PARITY.md index dae5709d..dae84f69 100644 --- a/docs/SQL_PARITY.md +++ b/docs/SQL_PARITY.md @@ -65,10 +65,11 @@ session apiece to fix properly, so **discovery is paused and the effort moves to picking them off**. Widen the corpus again when the open list is short, or opportunistically when a fix needs a case that doesn't exist yet. -Corpus coverage today: tiers 01–10, **230 cases** (197 AGREE / 16 DIFFER / -12 GAP / 1 OURS_ONLY / 4 BOTH_ERR as of 2026-10-03, after R13 slice 6 moved -HAVING and `IIF` onto the one truth rule, finishing [P62](#p62), and closed -[P52](#p52) on every join path — which uncovered [P63](#p63), open; [P51](#p51) +Corpus coverage today: tiers 01–10, **233 cases** (202 AGREE / 14 DIFFER / +12 GAP / 1 OURS_ONLY / 4 BOTH_ERR as of 2026-10-10, after [P63](#p63) and +[P64](#p64) fixed the single-equality hash join's RIGHT layout and its operand +routing; before that R13 slice 6 moved HAVING and `IIF` onto the one truth +rule, finishing [P62](#p62), and closed [P52](#p52) on every join path; [P51](#p51) open, [P53](#p53) open at low priority). The largest single movement so far remains the 2026-09-05 NULL-ordering slice, which closed [P13](#p13) stage 2 and [P17](#p17) together — eleven cases in one change. **Tier 10 (aggregate & NULL edges) is still @@ -95,7 +96,7 @@ Suggested fix order, by silent blast radius: | ~~9e~~ | ~~[P41](#p41) `MODE` tie-break is random per run~~ | ✅ **Fixed 2026-09-06** — 152 → **156 AGREE** (four new cases). Small, as predicted, but not where it was filed: the named `ModeState` was a *shadowed* implementation and fixing it moved nothing. Reference does specify a rule and it is **first-occurrence**, not the "smallest value wins" this row proposed. Unblocked both example files, now FORMAL. Spun off [P42](#p42), [P43](#p43), [R12](ENGINE_REFACTORING.md#r12) | | ~~9g~~ | ~~[P46](#p46) column-vs-column `WHERE` returns 0 rows~~ | ✅ **Fixed 2026-09-13** — 157 → **168 AGREE**. Wider than filed: *every* non-literal right-hand operand read as NULL (comparisons, `IN` items, `BETWEEN` bounds), and a literal on the left errored (`WHERE 1=0`). One resolver for all operands closed it. Did **not** need [P48](#p48) alongside, as this row predicted — the fix kept the comparison in the WHERE evaluator. Spun off [P49](#p49) (correlated outer refs bind to the inner table), accepted knowingly | | **NEXT** | [R13](ENGINE_REFACTORING.md#r13) one expression evaluator — **the active workstream from 2026-09-13** | **Slices 1–2 done 2026-09-13** (value evaluator three-valued, 168 → 174 AGREE, closed P48/P50); **slice 3 done 2026-09-14** (one construction path; case-insensitive mode now honoured outside WHERE, parity unmoved); **slice 4 in progress** (WHERE arms delegate, one operator per commit: BETWEEN/IN 2026-09-15, comparisons 2026-09-18 closing [P54](#p54), IS NULL and LIKE 2026-09-19 closing [P55](#p55), 175 → 179 AGREE; every WHERE operator now delegates; **finished 2026-10-01** with AND/OR/NOT/CASE, closing [P61](#p61) and [P62](#p62) under [D4](#d4), 183 → 186 AGREE); slice 5 (method calls) done 2026-09-26; slice 6 in progress (2026-10-03: pin, then HAVING and `IIF` onto the one truth rule, finishing P62, then JOIN NULL keys, closing [P52](#p52), 191 → 197 AGREE; retiring the WHERE evaluator left). Not a finding but the reason several are cheap to fix only once. WHERE and the value evaluator implement every boolean operator separately with different NULL rules, so the same predicate answers differently in WHERE, HAVING, SELECT and JOIN ON. **Working rule:** a parity fix that touches expression evaluation lands as an R13 slice, not a patch. [P52](#p52) (NULL join keys) is slice 6 | -| **NEXT** (beside R13) | [P63](#p63) single-condition `RIGHT JOIN` swaps `a.*` and `b.*` | Found 2026-10-03 by R13 slice 6. Silent, wrong values on *every* row, and every single-equality RIGHT JOIN is affected — the worst shape there is. Outside R13 (result assembly, not evaluation), so its own PR; pinned already | +| ~~beside R13~~ | ~~[P63](#p63) single-condition `RIGHT JOIN` swaps `a.*` and `b.*`~~ | ✅ **Fixed 2026-10-10** — 197 → **202 AGREE**. A `hash_join_right` sharing P8's column layout; the fix's own test found [P64](#p64) (the hash path routed operands by position, not alias — INNER and LEFT too), closed in the same PR | | then | [P60](#p60) `GROUP BY` unknown column → NULL group; [P58](#p58) JOIN ON half | Filed 2026-10-01. P60 is silent and a one-line cause, so cheap; P58's join half is a resolver fix — take it as the first slice of [R14](ENGINE_REFACTORING.md#r14) rather than a patch. [P59](#p59) (quoted alias) rides along if convenient | | then | [P47](#p47) `MIN`/`MAX` ranked by type | Silent and cheap, and outside the evaluator (aggregate registries — confirm the live one first, [R12](ENGINE_REFACTORING.md#r12)), so it can land alongside R13 without breaking the working rule | | then | [P14](#p14), [P20](#p20), [P23](#p23) | Smaller, self-contained, decisions already taken. Was row 9b, then NEXT until the 2026-09-12 findings displaced it. **Check each against the R13 working rule first** — P20 (`\|\|` with NULL) is an operator in the value evaluator | @@ -371,7 +372,8 @@ annotation be removed. - **Scope note:** Single-condition RIGHT joins (the hash path) always AGREEd, so this was confined to the multi-condition nested-loop RIGHT path. **Wrong, found 2026-10-03:** there was no single-condition RIGHT JOIN case - to AGREE — the hash path has the same fault, filed as [P63](#p63). + to AGREE — the hash path has the same fault, filed as [P63](#p63) (fixed + 2026-10-10). - **Fix:** Added a dedicated `nested_loop_join_right_multi` in `hash_join.rs` instead of reusing the swapped LEFT builder. It emits result columns in `[FROM, joined]` order (matching INNER/LEFT), keeps the FROM table's qualified @@ -2442,7 +2444,8 @@ out of date. - **Status:** ✅ FIXED 2026-10-03 by [R13](ENGINE_REFACTORING.md#r13) slice 6 — on every path. Six of the seven cases below DIFFER → AGREE (191 → 197); the seventh, the hash RIGHT join, now returns DuckDB's 12 rows but still - differs because of [P63](#p63), which it uncovered. + differs because of [P63](#p63), which it uncovered (AGREE since P63's fix, + 2026-10-10). - **Fix:** the nested loop's `compare_values` asks the predicate-layer `compare_trilean` and keeps a pair only when it is TRUE; the hash path never indexes a NULL key, so a NULL probe finds nothing and LEFT emits its row @@ -2749,12 +2752,14 @@ out of date. two evaluators could not delegate to each other without choosing one rule. ### P63 — A single-condition RIGHT JOIN swaps the two tables' columns -- **Status:** 🔴 OPEN — silent, wrong values on every row. Pinned; outside +- **Status:** ✅ FIXED 2026-10-10 — both cases DIFFER → AGREE. Outside [R13](ENGINE_REFACTORING.md#r13) (join result assembly, not evaluation), so - its own fix. -- **Corpus:** `04_joins.toml :: right_join_single_condition` - (`expect = "DIFFER"`), and `10_aggregate_nulls.toml :: - right_join_on_null_key_both_sides`, whose P52 half is fixed. + its own PR, which also closed [P64](#p64). +- **Corpus:** `04_joins.toml :: right_join_single_condition`, and + `10_aggregate_nulls.toml :: right_join_on_null_key_both_sides`, whose P52 + half was fixed first. Rust regression tests beside P8's in + `tests/join_operand_order_tests.rs`, over an asymmetric `edges` table — the + hash path checked against the nested loop as well as against expected rows. - **Observed:** `null_edges a RIGHT JOIN null_edges b ON a.partner_id = b.id` returns 12 rows, as DuckDB does, but `a.*` holds the `b` row and `b.*` the `a` row: for `b.id = 1` we return `a.id = 3` (b's own partner) where DuckDB @@ -2774,11 +2779,46 @@ out of date. Routing through `right_multi` would be correct but lose the hash path. Pin with a Rust regression test beside P8's in `tests/join_operand_order_tests.rs`. +- **Fix (as directed):** `hash_join_right` indexes the FROM table and probes + it with each joined row in order, NULL-filling the FROM side when nothing + matches; NULL keys are never indexed (P52). Its result table comes from + `right_join_result_table`, factored out of `nested_loop_join_right_multi`, + so the two RIGHT builders cannot drift apart on layout again. - **Found:** 2026-10-03, R13 slice 6 (P52's RIGHT case stayed DIFFER after the NULL fix). --- +### P64 — A single-equality join reads `b.x = a.y` as `a.x = b.y` +- **Status:** ✅ FIXED 2026-10-10, in P63's PR — three new cases, all DIFFER + on main, AGREE after. +- **Corpus:** `04_joins.toml :: {inner,left,right}_join_single_condition_joined_side_first`. +- **Observed:** `null_edges a JOIN null_edges b ON b.id = a.partner_id` + returned `(1, 2)`, `(3, 1)`… — the rows of `a.id = b.partner_id`. Writing + the same predicate as `a.partner_id = b.id` gave the right answer. INNER, + LEFT and RIGHT alike. +- **Cause:** [P7](#p7)'s fault on the path P7 did not reach. `execute_join` + resolves a simple column condition by taking the written-left name in the + FROM table and the written-right name in the joined table, and the lookup + drops the qualifier — so when both tables have both columns (a self-join, + or two tables sharing column names) each name resolves in the wrong table, + silently. When the names exist on one side only, the answer was already + right (checked 2026-10-10 with a CTE renaming `id` as the joined table), so + the fault needs both tables to hold both columns. P7 fixed the nested + loop's *evaluation*, but every simple-column condition, multi-condition + ones included, still passes through this resolver first. +- **Why it hid:** the P7 cases all have two conditions, which take the + nested loop for evaluation; their resolution was self-join-symmetric. + Found by P63's regression test, which wrote the predicate both ways. +- **Fix:** before resolving, route each operand by alias with P7's + `operand_uses_right`; a condition written joined-side first is swapped, + and its operator reversed (`reverse_operator`, which P63's change had + left unused). An unqualified operand keeps its written position, as in + the nested loop. +- **Found:** 2026-10-10, fixing P63. + +--- + ## Deferred / won't fix (intentional) ### D1 — Recursive CTEs (`WITH RECURSIVE`)