Skip to content

Commit

Permalink
fix(resolver): do not copy sort when flattening a join or append tran…
Browse files Browse the repository at this point in the history
…sform (#5066)

Signed-off-by: Luka Peschke <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
lukapeschke and pre-commit-ci[bot] authored Dec 21, 2024
1 parent 07728bb commit d05b6bd
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 1 deletion.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

**Fixes**:

- Sort steps in sub-pipelines no longer cause a column lookup error
(@lukapeschke, #5066)

**Documentation**:

**Web**:
Expand All @@ -18,6 +21,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.
Expand Down
21 changes: 20 additions & 1 deletion prqlc/prqlc/src/semantic/resolver/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?,
Expand Down
83 changes: 83 additions & 0 deletions prqlc/prqlc/tests/integration/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d05b6bd

Please sign in to comment.