Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: revert sorts on all CTEs (in PQ postprocessing) #5128

Merged
merged 1 commit into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions prqlc/prqlc/src/sql/pq/postprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ impl PqMapper<RelationExpr, RelationExpr, (), ()> for SortingInference<'_> {
transforms: Vec<SqlTransform<RelationExpr, ()>>,
) -> Result<Vec<SqlTransform<RelationExpr, ()>>> {
let mut sorting = Vec::new();
let mut has_sort_transform = false;

let mut result = Vec::with_capacity(transforms.len() + 1);

Expand Down Expand Up @@ -128,7 +127,6 @@ impl PqMapper<RelationExpr, RelationExpr, (), ()> for SortingInference<'_> {
// just store sorting and don't emit Sort
SqlTransform::Sort(expr) => {
sorting.clone_from(&expr);
has_sort_transform = true;
continue;
}

Expand Down Expand Up @@ -159,13 +157,11 @@ impl PqMapper<RelationExpr, RelationExpr, (), ()> for SortingInference<'_> {
}
}

if has_sort_transform {
// now revert the sort columns so that the output
// sorting reflects the input column cids, needed to
// ensure proper column reference lookup in the final
// steps
sorting = CidRedirector::revert_sorts(sorting, &mut self.ctx.anchor);
}
// now revert the sort columns so that the output
// sorting reflects the input column cids, needed to
// ensure proper column reference lookup in the final
// steps
sorting = CidRedirector::revert_sorts(sorting, &mut self.ctx.anchor);
}

// remember sorting for this pipeline
Expand Down
8 changes: 8 additions & 0 deletions prqlc/prqlc/tests/integration/queries/sort_3.prql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from [{track_id=0, album_id=1, genre_id=2}]
select { AA=track_id, album_id, genre_id }
sort AA
join side:left [{album_id=1, album_title="Songs"}] (==album_id)
select { AA, AT = album_title ?? "unknown", genre_id }
filter AA < 25
join side:left [{genre_id=1, genre_title="Rock"}] (==genre_id)
select { AA, AT, GT = genre_title ?? "unknown" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
---
source: prqlc/prqlc/tests/integration/queries.rs
expression: "from [{track_id=0, album_id=1, genre_id=2}]\nselect { AA=track_id, album_id, genre_id }\nsort AA\njoin side:left [{album_id=1, album_title=\"Songs\"}] (==album_id)\nselect { AA, AT = album_title ?? \"unknown\", genre_id }\nfilter AA < 25\njoin side:left [{genre_id=1, genre_title=\"Rock\"}] (==genre_id)\nselect { AA, AT, GT = genre_title ?? \"unknown\" }\n"
input_file: prqlc/prqlc/tests/integration/queries/sort_3.prql
---
WITH table_0 AS (
SELECT
0 AS track_id,
1 AS album_id,
2 AS genre_id
),
table_5 AS (
SELECT
track_id AS "AA",
genre_id,
album_id
FROM
table_0
),
table_1 AS (
SELECT
1 AS album_id,
'Songs' AS album_title
),
table_4 AS (
SELECT
table_5."AA",
COALESCE(table_1.album_title, 'unknown') AS "AT",
table_5.genre_id
FROM
table_5
LEFT JOIN table_1 ON table_5.album_id = table_1.album_id
),
table_3 AS (
SELECT
"AA",
"AT",
genre_id
FROM
table_4
WHERE
"AA" < 25
),
table_2 AS (
SELECT
1 AS genre_id,
'Rock' AS genre_title
)
SELECT
table_3."AA",
table_3."AT",
COALESCE(table_2.genre_title, 'unknown') AS "GT"
FROM
table_3
LEFT JOIN table_2 ON table_3.genre_id = table_2.genre_id
ORDER BY
table_3."AA"
Loading