Skip to content

Commit

Permalink
Throw an error when ordering by aggregation function over an empty path
Browse files Browse the repository at this point in the history
  • Loading branch information
plcplc committed Nov 14, 2023
1 parent 991f25f commit cd0197c
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,14 @@ fn translate_order_by_target_for_column(
)?;

if path.is_empty() {
// if we got an aggregation function, the query does not make sense.
match function {
None => Ok(())?,
Some(func) => Err(Error::NotSupported(format!(
"Cannot order by aggregation function '{func}' on the same table"
)))?,
};

// if there were no relationship columns, we don't need to build a query, just return the column.
let table = env.lookup_collection(&root_and_current_tables.current_table.name)?;
let selected_column = table.lookup_column(&column_name)?;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"collection": "Album",
"query": {
"fields": {
"Name": {
"type": "column",
"column": "Title",
"arguments": {}
}
},
"order_by": {
"elements": [
{
"target": {
"type": "single_column_aggregate",
"column": "TopRadioChartPlacement",
"function": "impossible",
"path": [ ]
},
"order_direction": "desc"
}
]
},
"limit": 3
},
"arguments": {},
"collection_relationships": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"tables": {
"Album": {
"schemaName": "public",
"tableName": "Album",
"columns": {
"AlbumId": {
"name": "AlbumId",
"type": "int4"
},
"Title": {
"name": "Title",
"type": "varchar"
},
"TopRadioChartPlacement": {
"name": "TopRadioChartPlacement",
"type": "int4",
"nullable": "nullable"
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/query-engine/translation/tests/tests.rs
expression: "format!(\"{result:?}\")"
---
Err(NotSupported("Cannot order by aggregation function 'impossible' on the same table"))
6 changes: 6 additions & 0 deletions crates/query-engine/translation/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ fn sorting_by_nested_relationship_count() {
insta::assert_snapshot!(result);
}

#[test]
fn sorting_by_no_relationship_aggregate() {
let result = common::test_translation("sorting_by_no_relationship_aggregate");
insta::assert_snapshot!(format!("{result:?}"));
}

#[test]
fn sorting_by_relationship_count_with_predicate() {
let result = common::test_translation("sorting_by_relationship_count_with_predicate").unwrap();
Expand Down

0 comments on commit cd0197c

Please sign in to comment.