Skip to content

Commit 33498ae

Browse files
committed
feat(convert): run FULL JOIN USING after a JOIN without ON left to right
Snowflake binds a later FULL JOIN ... USING to the whole join so far, as it does a later ON: after a join without ON, the recorded cases return the left-to-right row count, not the nested one. The conversion no longer refuses that shape. RIGHT JOIN with USING and RIGHT JOIN without ON stay refused. The recorded cases are in the pinned evidence, and the translation results baseline gains their two statements.
1 parent 2082f04 commit 33498ae

4 files changed

Lines changed: 19 additions & 40 deletions

File tree

‎coverage/translation/results-baseline.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6649,6 +6649,8 @@
66496649
"relational/cross_join_then_outer_join.toml#cross_join_then_full_join_on_first_table": "matching",
66506650
"relational/cross_join_then_outer_join.toml#cross_join_then_right_join_on": "matching",
66516651
"relational/cross_join_then_outer_join.toml#join_without_on_then_full_join_on": "matching",
6652+
"relational/cross_join_then_outer_join.toml#join_without_on_then_full_join_using": "matching",
6653+
"relational/cross_join_then_outer_join.toml#join_without_on_then_right_join_using": "refused",
66526654
"relational/fixture_shapes.toml#not_in_subquery_filtered_not_null_in_case": "matching_rejection",
66536655
"relational/fixture_shapes.toml#not_in_subquery_in_case": "matching_rejection",
66546656
"relational/fixture_shapes.toml#not_in_subquery_of_literal_rows_in_case": "matching",

‎crates/sqlquack-convert/src/snowflake/catalogue/constants.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ pub const RULES: &[RuleSpec] = &[
149149
"join:left",
150150
"join:full",
151151
],
152-
conditions: "No condition: CROSS JOIN, a comma, or JOIN, INNER JOIN, LEFT JOIN or FULL JOIN without ON, which Snowflake runs as a cross join, with no rows when either side is empty, as recorded; a later RIGHT or FULL JOIN with ON applies to the whole join before it, as recorded; one before a FULL JOIN with USING refuses, as that is unrecorded.",
152+
conditions: "No condition: CROSS JOIN, a comma, or JOIN, INNER JOIN, LEFT JOIN or FULL JOIN without ON, which Snowflake runs as a cross join, with no rows when either side is empty, as recorded; a later RIGHT or FULL JOIN with ON, or FULL JOIN with USING, applies to the whole join before it, as recorded.",
153153
failures: &[],
154154
},
155155
RuleSpec {

‎crates/sqlquack-convert/src/snowflake/conversion/_helpers/queries/sources.rs‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,10 @@ pub(crate) fn convert_from(
123123
sql.push_str(&source_sql);
124124
relations.push(relation);
125125
}
126-
let joins = list(select, "joins");
127-
for (index, join) in joins.iter().enumerate() {
126+
for join in list(select, "joins") {
128127
let before = events();
129128
let merging = RefCell::new(None);
130-
let using_follows = joins[index + 1..].iter().any(full_join_using);
131-
let joined = convert_join(join, &relations, (outer, &merging, using_follows), context)
129+
let joined = convert_join(join, &relations, (outer, &merging), context)
132130
.map_err(|refusal| context.locator.placed(refusal, join));
133131
let source = join.get("this").unwrap_or(&Value::Null);
134132
let (join_sql, relation) = recovered(joined, source, (before, relations.len()))?;
@@ -170,19 +168,10 @@ struct Merging {
170168
relation: Option<ScopeRelation>,
171169
}
172170

173-
/// Whether `join` is a FULL JOIN with USING, whose binding after a join without ON is unrecorded.
174-
fn full_join_using(join: &Value) -> bool {
175-
join.get("kind").and_then(Value::as_str) == Some("Full")
176-
&& join
177-
.get("using")
178-
.and_then(Value::as_array)
179-
.is_some_and(|using| !using.is_empty())
180-
}
181-
182171
fn convert_join(
183172
join: &Value,
184173
earlier: &[ScopeRelation],
185-
(outer, merging, using_follows): (Option<&Scope<'_>>, &RefCell<Option<Merging>>, bool),
174+
(outer, merging): (Option<&Scope<'_>>, &RefCell<Option<Merging>>),
186175
context: &Context<'_>,
187176
) -> Result<(String, ScopeRelation), Refusal> {
188177
let join_construct = node_construct("join");
@@ -234,14 +223,6 @@ fn convert_join(
234223
("Left", Some(_)) => "LEFT JOIN",
235224
("Right", Some(_)) => "RIGHT JOIN",
236225
("Full", Some(_)) => "FULL JOIN",
237-
("Inner" | "Left" | "Full", None) if using_follows => {
238-
return Err(missing(
239-
Construct::new(ConstructFamily::Join, kind.to_ascii_lowercase()),
240-
format!(
241-
"a {kind} join without ON before a FULL JOIN with USING, which no receipt records"
242-
),
243-
));
244-
}
245226
("Cross" | "Inner" | "Left" | "Full", None) => "CROSS JOIN",
246227
("Implicit", None) => ",",
247228
_ => {

‎crates/sqlquack/tests/cli/test_joins.rs‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ fn given_join_without_on_when_converted_and_run_on_duckdb_then_it_is_a_cross_joi
5252
LEFT JOIN (SELECT 1 AS k) r RIGHT JOIN (SELECT 1 AS k2 UNION ALL SELECT 9) w ON w.k2 = r.k",
5353
expected_value: "3",
5454
},
55+
ValueTestCase {
56+
description: "a FULL JOIN with USING after a JOIN without ON applies to the whole join so far, as recorded",
57+
sql: "SELECT TO_VARCHAR(COUNT(*)) AS n FROM (SELECT 1 AS x UNION ALL SELECT 2) a \
58+
JOIN (SELECT 1 AS k) r FULL JOIN (SELECT 1 AS k UNION ALL SELECT 9) w USING (k)",
59+
expected_value: "3",
60+
},
5561
ValueTestCase {
5662
description: "FULL JOIN without ON over two non-empty sides pairs every row",
5763
sql: "SELECT TO_VARCHAR(COUNT(*)) AS n FROM (SELECT 1 AS x UNION ALL SELECT 2) l \
@@ -71,23 +77,13 @@ fn given_join_without_on_when_converted_and_run_on_duckdb_then_it_is_a_cross_joi
7177

7278
#[test]
7379
fn given_a_join_without_on_snowflake_has_not_recorded_when_converting_then_it_refuses() {
74-
let test_cases = [
75-
ConvertTestCase {
76-
description: "RIGHT JOIN without ON, which no receipt records, refuses",
77-
sql: "SELECT l.x AS x FROM (SELECT 1 AS x) l RIGHT JOIN (SELECT 1 AS y) r",
78-
flags: &[],
79-
mention: "no rule converts a Right join of this shape",
80-
expected_outcome: convert_outcome(1, false),
81-
},
82-
ConvertTestCase {
83-
description: "a JOIN without ON before a FULL JOIN with USING, which no receipt records, refuses",
84-
sql: "SELECT a.x AS x FROM (SELECT 1 AS x) a JOIN (SELECT 1 AS k) r \
85-
FULL JOIN (SELECT 1 AS k) w USING (k)",
86-
flags: &[],
87-
mention: "before a FULL JOIN with USING",
88-
expected_outcome: convert_outcome(1, false),
89-
},
90-
];
80+
let test_cases = [ConvertTestCase {
81+
description: "RIGHT JOIN without ON, which no receipt records, refuses",
82+
sql: "SELECT l.x AS x FROM (SELECT 1 AS x) l RIGHT JOIN (SELECT 1 AS y) r",
83+
flags: &[],
84+
mention: "no rule converts a Right join of this shape",
85+
expected_outcome: convert_outcome(1, false),
86+
}];
9187
for test_case in test_cases {
9288
assert_eq!(
9389
run_convert(test_case.sql, test_case.flags, test_case.mention),

0 commit comments

Comments
 (0)