From 1a5e3a7215ca7c51fe30e6753b841184564ac104 Mon Sep 17 00:00:00 2001 From: Luka Peschke Date: Fri, 20 Dec 2024 10:35:34 +0100 Subject: [PATCH 1/3] fix(resolver): do not copy sort when flattening a join or append transform closes #4063 #4947 Signed-off-by: Luka Peschke --- prqlc/prqlc/src/semantic/resolver/flatten.rs | 21 ++++- prqlc/prqlc/tests/integration/sql.rs | 83 ++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/prqlc/prqlc/src/semantic/resolver/flatten.rs b/prqlc/prqlc/src/semantic/resolver/flatten.rs index 7c9afb00a4f9..b96827adc67c 100644 --- a/prqlc/prqlc/src/semantic/resolver/flatten.rs +++ b/prqlc/prqlc/src/semantic/resolver/flatten.rs @@ -131,12 +131,31 @@ impl PlFold for Flattener { kind => (self.fold_expr(*t.input)?, fold_transform_kind(self, kind)?), }; + // In case we're appending or joining another pipeline, we do not want to apply the + // sub-pipeline's sort, as it may result in column lookup errors. Without this, we + // would try to join on `album_id` in the outer pipeline of the following query, but + // the column does not exist + // + // from artists + // join side:left ( + // from albums + // sort {`album_id`} + // derive {`album_name` = `name`} + // select {`artist_id`, `album_name`} + // ) (this.id == that.artist_id) + let sort = if matches!(kind, TransformKind::Join { .. } | TransformKind::Append(_)) + { + vec![] + } else { + self.sort.clone() + }; + ExprKind::TransformCall(TransformCall { input: Box::new(input), kind: Box::new(kind), partition: self.partition.clone(), frame: self.window.clone(), - sort: self.sort.clone(), + sort, }) } kind => self.fold_expr_kind(kind)?, diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index 8d86aa36074e..098f6640aca4 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -885,6 +885,89 @@ fn test_intersect_07() { ); } +#[test] +fn test_sort_in_nested_join() { + assert_snapshot!(compile(r#" + from albums + join side:left ( + from artists + sort {-`artist-id`} + take 10 + ) (this.artist_id == that.artist_id) | take 10 + "#).unwrap(), + @r#" + WITH table_0 AS ( + SELECT + * + FROM + artists + ORDER BY + "artist-id" DESC + LIMIT + 10 + ) + SELECT + albums.*, + table_0.* + FROM + albums + LEFT JOIN table_0 ON albums.artist_id = table_0.artist_id + LIMIT + 10 + "# + ); +} + +#[test] +fn test_sort_in_nested_append() { + assert_snapshot!(compile(r#" + from `albums` + select { `album_id`, `title` } + sort {+`album_id`} + take 2 + append ( + from `albums` + select { `album_id`, `title` } + sort {-`album_id`} + take 2 + ) + "#).unwrap(), + @r#" + WITH table_0 AS ( + SELECT + album_id, + title + FROM + albums + ORDER BY + album_id DESC + LIMIT + 2 + ) + SELECT + * + FROM + ( + SELECT + album_id, + title + FROM + albums + ORDER BY + album_id + LIMIT + 2 + ) AS table_1 + UNION + ALL + SELECT + * + FROM + table_0 + "# + ); +} + #[test] fn test_rn_ids_are_unique() { // this is wrong, output will have duplicate y_id and x_id From 451ad5879e52fa921f426f2bf94cf38bb9b4e9c2 Mon Sep 17 00:00:00 2001 From: Luka Peschke Date: Sat, 21 Dec 2024 12:34:06 +0100 Subject: [PATCH 2/3] docs: update changelog Signed-off-by: Luka Peschke --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9275f7c87e43..29f45d786935 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ **Fixes**: +- Sort steps in sub-pipelines no longer cause a column lookup error (@lukapeschke, #5066) + **Documentation**: **Web**: @@ -18,6 +20,8 @@ **New Contributors**: +- @lukapeschke, with #5066 + ## 0.13.2 0.13.2 is a tiny release to fix an issue publishing 0.13.1 to crates.io. From fd41d4705cfa04b2567ef418f8e6cd84e405e903 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 21 Dec 2024 11:34:35 +0000 Subject: [PATCH 3/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29f45d786935..c9fb01c5bbaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ **Fixes**: -- Sort steps in sub-pipelines no longer cause a column lookup error (@lukapeschke, #5066) +- Sort steps in sub-pipelines no longer cause a column lookup error + (@lukapeschke, #5066) **Documentation**: