From 21722c1480ec56a8b03858b7d7267b8935a90750 Mon Sep 17 00:00:00 2001 From: Karl Gutwin Date: Tue, 21 May 2024 23:39:43 -0400 Subject: [PATCH 1/6] Allow join side: to take an ident that resolves to a literal --- .../prqlc/src/semantic/resolver/transforms.rs | 31 ++++++-- prqlc/prqlc/tests/integration/sql.rs | 74 +++++++++++++++++++ 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/prqlc/prqlc/src/semantic/resolver/transforms.rs b/prqlc/prqlc/src/semantic/resolver/transforms.rs index f59dd01ee81d..6cd6e2de4279 100644 --- a/prqlc/prqlc/src/semantic/resolver/transforms.rs +++ b/prqlc/prqlc/src/semantic/resolver/transforms.rs @@ -101,7 +101,9 @@ impl Resolver<'_> { let side = { let span = side.span; - let ident = side.try_cast(ExprKind::into_ident, Some("side"), "ident")?; + let ident = + side.clone() + .try_cast(ExprKind::into_ident, Some("side"), "ident")?; match ident.to_string().as_str() { "inner" => JoinSide::Inner, "left" => JoinSide::Left, @@ -109,12 +111,27 @@ impl Resolver<'_> { "full" => JoinSide::Full, found => { - return Err(Error::new(Reason::Expected { - who: Some("`side`".to_string()), - expected: "inner, left, right or full".to_string(), - found: found.to_string(), - }) - .with_span(span)) + let folded = self.fold_expr(side)?.try_cast( + ExprKind::into_literal, + Some("side"), + "string literal", + )?; + + match folded.to_string().as_str() { + "\"inner\"" => JoinSide::Inner, + "\"left\"" => JoinSide::Left, + "\"right\"" => JoinSide::Right, + "\"full\"" => JoinSide::Full, + + _ => { + return Err(Error::new(Reason::Expected { + who: Some("`side`".to_string()), + expected: "inner, left, right or full".to_string(), + found: found.to_string(), + }) + .with_span(span)) + } + } } } }; diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index 2e25d8a028e1..76fa4ce4ff62 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -2385,6 +2385,80 @@ fn test_join() { compile("from x | join y {==x.id}").unwrap_err(); } +#[test] +fn test_join_side_literal() { + assert_snapshot!((compile(r###" + let my_side = "right" + + from x + join y (==id) side:my_side + "###).unwrap()), @r###" + SELECT + x.*, + y.* + FROM + x + RIGHT JOIN y ON x.id = y.id + "###); +} + +#[test] +fn test_join_side_literal_err() { + assert_snapshot!((compile(r###" + let my_side = 42 + + from x + join y (==id) side:my_side + "###).unwrap_err()), @r###" + Error: + ╭─[:5:24] + │ + 5 │ join y (==id) side:my_side + │ ───┬─── + │ ╰───── `side` expected inner, left, right or full, but found my_side + ───╯ + "###); +} + +#[test] +fn test_join_side_literal_via_func() { + assert_snapshot!((compile(r###" + let my_join = func m c s :"right" tbl -> ( + join side:_param.s m (c == that.k) tbl + ) + + from x + my_join default_db.y this.id s:"left" + "###).unwrap()), @r###" + SELECT + x.*, + y.* + FROM + x + LEFT JOIN y ON x.id = y.k + "###); +} + +#[test] +fn test_join_side_literal_via_func_err() { + assert_snapshot!((compile(r###" + let my_join = func m c s :"right" tbl -> ( + join side:_param.s m (c == that.k) tbl + ) + + from x + my_join default_db.y this.id s:"four" + "###).unwrap_err()), @r###" + Error: + ╭─[:3:25] + │ + 3 │ join side:_param.s m (c == that.k) tbl + │ ─┬ + │ ╰── `side` expected inner, left, right or full, but found _param.s + ───╯ + "###); +} + #[test] fn test_from_json() { // Test that the SQL generated from the JSON of the PRQL is the same as the raw PRQL From df98d1a3ee7b7a7786c66ef04091105c7e676a2a Mon Sep 17 00:00:00 2001 From: Karl Gutwin Date: Wed, 22 May 2024 10:07:16 -0400 Subject: [PATCH 2/6] comment and changelog --- CHANGELOG.md | 1 + prqlc/prqlc/src/semantic/resolver/transforms.rs | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d78b9b3c1de..df539d253dc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ generates Markdown documentation from `.prql` files. (@vanillajonathan, #4152). - Add `prqlc lex` command to the CLI (@max-sixty) +- Join `side` parameter can take a reference that resolves to a literal (@kgutwin, #4499) **Fixes**: diff --git a/prqlc/prqlc/src/semantic/resolver/transforms.rs b/prqlc/prqlc/src/semantic/resolver/transforms.rs index 6cd6e2de4279..ea41a167e4c6 100644 --- a/prqlc/prqlc/src/semantic/resolver/transforms.rs +++ b/prqlc/prqlc/src/semantic/resolver/transforms.rs @@ -104,6 +104,8 @@ impl Resolver<'_> { let ident = side.clone() .try_cast(ExprKind::into_ident, Some("side"), "ident")?; + + // first try to match the raw ident string as a bare word match ident.to_string().as_str() { "inner" => JoinSide::Inner, "left" => JoinSide::Left, @@ -111,6 +113,10 @@ impl Resolver<'_> { "full" => JoinSide::Full, found => { + // if that fails, fold the ident and try + // treating the result as a literal + // this allows the join side to be passed as a + // function parameter let folded = self.fold_expr(side)?.try_cast( ExprKind::into_literal, Some("side"), From 65bbdfd7f91565092eba50739f649baa83a68766 Mon Sep 17 00:00:00 2001 From: Karl Gutwin Date: Thu, 23 May 2024 16:16:06 -0400 Subject: [PATCH 3/6] update comment --- prqlc/prqlc/src/semantic/resolver/transforms.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/prqlc/prqlc/src/semantic/resolver/transforms.rs b/prqlc/prqlc/src/semantic/resolver/transforms.rs index ea41a167e4c6..a8b299cc329f 100644 --- a/prqlc/prqlc/src/semantic/resolver/transforms.rs +++ b/prqlc/prqlc/src/semantic/resolver/transforms.rs @@ -113,10 +113,9 @@ impl Resolver<'_> { "full" => JoinSide::Full, found => { - // if that fails, fold the ident and try - // treating the result as a literal - // this allows the join side to be passed as a - // function parameter + // if that fails, fold the ident and try treating the result as a literal + // this allows the join side to be passed as a function parameter + // NOTE: this is temporary, pending discussions and implementation, tracked in #4501 let folded = self.fold_expr(side)?.try_cast( ExprKind::into_literal, Some("side"), From f82db4351977644354816140fcb2026454caf9b3 Mon Sep 17 00:00:00 2001 From: Karl Gutwin Date: Thu, 23 May 2024 16:21:16 -0400 Subject: [PATCH 4/6] report resolved value in err msg --- prqlc/prqlc/src/semantic/resolver/transforms.rs | 4 ++-- prqlc/prqlc/tests/integration/sql.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/prqlc/prqlc/src/semantic/resolver/transforms.rs b/prqlc/prqlc/src/semantic/resolver/transforms.rs index a8b299cc329f..69aa8ad64b6d 100644 --- a/prqlc/prqlc/src/semantic/resolver/transforms.rs +++ b/prqlc/prqlc/src/semantic/resolver/transforms.rs @@ -112,7 +112,7 @@ impl Resolver<'_> { "right" => JoinSide::Right, "full" => JoinSide::Full, - found => { + _ => { // if that fails, fold the ident and try treating the result as a literal // this allows the join side to be passed as a function parameter // NOTE: this is temporary, pending discussions and implementation, tracked in #4501 @@ -132,7 +132,7 @@ impl Resolver<'_> { return Err(Error::new(Reason::Expected { who: Some("`side`".to_string()), expected: "inner, left, right or full".to_string(), - found: found.to_string(), + found: folded.to_string(), }) .with_span(span)) } diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index 76fa4ce4ff62..f89a9ce6e179 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -2415,7 +2415,7 @@ fn test_join_side_literal_err() { │ 5 │ join y (==id) side:my_side │ ───┬─── - │ ╰───── `side` expected inner, left, right or full, but found my_side + │ ╰───── `side` expected inner, left, right or full, but found 42 ───╯ "###); } @@ -2454,7 +2454,7 @@ fn test_join_side_literal_via_func_err() { │ 3 │ join side:_param.s m (c == that.k) tbl │ ─┬ - │ ╰── `side` expected inner, left, right or full, but found _param.s + │ ╰── `side` expected inner, left, right or full, but found "four" ───╯ "###); } From e89308f0cd4bb3a6e4d20e6a58e0327b2824da56 Mon Sep 17 00:00:00 2001 From: Karl Gutwin Date: Thu, 23 May 2024 17:36:21 -0400 Subject: [PATCH 5/6] Update CHANGELOG.md Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df539d253dc9..b03f7dcbfc6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ generates Markdown documentation from `.prql` files. (@vanillajonathan, #4152). - Add `prqlc lex` command to the CLI (@max-sixty) -- Join `side` parameter can take a reference that resolves to a literal (@kgutwin, #4499) +- Join `side` parameter can take a reference that resolves to a literal (note: this is an experimental feature which may change in the future) (@kgutwin, #4499) **Fixes**: From 76234863d93b3f796f4ee2ade65d264c5b799102 Mon Sep 17 00:00:00 2001 From: Karl Gutwin Date: Thu, 23 May 2024 17:47:52 -0400 Subject: [PATCH 6/6] pre-commit update --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b03f7dcbfc6f..20964900e158 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,9 @@ generates Markdown documentation from `.prql` files. (@vanillajonathan, #4152). - Add `prqlc lex` command to the CLI (@max-sixty) -- Join `side` parameter can take a reference that resolves to a literal (note: this is an experimental feature which may change in the future) (@kgutwin, #4499) +- Join `side` parameter can take a reference that resolves to a literal (note: + this is an experimental feature which may change in the future) (@kgutwin, + #4499) **Fixes**: