From fccc2d9a41e6cac9cd49a054630297442c78aeea Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Fri, 16 Aug 2024 23:25:27 -0400 Subject: [PATCH 1/5] remove group by and order by clauses that should not be there --- .../ndc-clickhouse/src/sql/query_builder.rs | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/crates/ndc-clickhouse/src/sql/query_builder.rs b/crates/ndc-clickhouse/src/sql/query_builder.rs index 9992671..6b890dc 100644 --- a/crates/ndc-clickhouse/src/sql/query_builder.rs +++ b/crates/ndc-clickhouse/src/sql/query_builder.rs @@ -165,34 +165,10 @@ impl<'r, 'c> QueryBuilder<'r, 'c> { vec![rowset_subquery.into_table_with_joins(vec![])] }; - let order_by = if self.request.variables.is_some() { - vec![OrderByExpr { - expr: Expr::CompoundIdentifier(vec![ - Ident::new_quoted("_vars"), - Ident::new_quoted("_varset_id"), - ]), - asc: Some(true), - nulls_first: None, - }] - } else { - vec![] - }; - - let group_by = if self.request.variables.is_some() { - vec![Expr::CompoundIdentifier(vec![ - Ident::new_quoted("_vars"), - Ident::new_quoted("_varset_id"), - ])] - } else { - vec![] - }; - Ok(Query::new() .with(with) .select(select) .from(from) - .group_by(group_by) - .order_by(order_by) .into_statement() .format("TabSeparatedRaw")) } From 204275c49c2f94861c205905d150a2fd8a6eae87 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Fri, 16 Aug 2024 23:26:14 -0400 Subject: [PATCH 2/5] update variables tests to test multiple sets, and remove goup by and order by clauses --- crates/ndc-clickhouse/tests/query_builder.rs | 4 ++ .../01_simple_predicate.request.json | 3 ++ .../01_simple_predicate.statement.sql | 8 +-- .../02_empty_variable_sets.statement.sql | 6 +-- .../03_variables/03_single_set.request.json | 53 +++++++++++++++++++ .../03_variables/03_single_set.statement.sql | 48 +++++++++++++++++ 6 files changed, 111 insertions(+), 11 deletions(-) create mode 100644 crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.request.json create mode 100644 crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.statement.sql diff --git a/crates/ndc-clickhouse/tests/query_builder.rs b/crates/ndc-clickhouse/tests/query_builder.rs index dd29a2d..9b1ce87 100644 --- a/crates/ndc-clickhouse/tests/query_builder.rs +++ b/crates/ndc-clickhouse/tests/query_builder.rs @@ -252,6 +252,10 @@ mod variables { async fn empty_variable_sets() -> Result<(), Box> { test_generated_sql("02_empty_variable_sets").await } + #[tokio::test] + async fn singe_set() -> Result<(), Box> { + test_generated_sql("03_single_set").await + } } #[cfg(test)] diff --git a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.request.json b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.request.json index eacd229..571946a 100644 --- a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.request.json +++ b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.request.json @@ -4,6 +4,9 @@ "variables": [ { "ArtistId": 1 + }, + { + "ArtistId": 2 } ], "query": { diff --git a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.statement.sql b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.statement.sql index c5451bf..f6a1260 100644 --- a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.statement.sql +++ b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/01_simple_predicate.statement.sql @@ -4,7 +4,7 @@ WITH "_vars" AS ( FROM format( JSONColumns, - '{"_varset_id":[1],"_var_ArtistId":[1]}' + '{"_varset_id":[1,2],"_var_ArtistId":[1,2]}' ) ) SELECT @@ -45,8 +45,4 @@ FROM ) AS "_row" GROUP BY "_row"."_varset_id" - ) AS "_rowset" ON "_vars"."_varset_id" = "_rowset"."_varset_id" -GROUP BY - "_vars"."_varset_id" -ORDER BY - "_vars"."_varset_id" ASC FORMAT TabSeparatedRaw; \ No newline at end of file + ) AS "_rowset" ON "_vars"."_varset_id" = "_rowset"."_varset_id" FORMAT TabSeparatedRaw; \ No newline at end of file diff --git a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/02_empty_variable_sets.statement.sql b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/02_empty_variable_sets.statement.sql index 57a507c..cd9a3a5 100644 --- a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/02_empty_variable_sets.statement.sql +++ b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/02_empty_variable_sets.statement.sql @@ -42,8 +42,4 @@ FROM ) AS "_row" GROUP BY "_row"."_varset_id" - ) AS "_rowset" ON "_vars"."_varset_id" = "_rowset"."_varset_id" -GROUP BY - "_vars"."_varset_id" -ORDER BY - "_vars"."_varset_id" ASC FORMAT TabSeparatedRaw; \ No newline at end of file + ) AS "_rowset" ON "_vars"."_varset_id" = "_rowset"."_varset_id" FORMAT TabSeparatedRaw; \ No newline at end of file diff --git a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.request.json b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.request.json new file mode 100644 index 0000000..eacd229 --- /dev/null +++ b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.request.json @@ -0,0 +1,53 @@ +{ + "$schema": "../../request.schema.json", + "collection": "Chinook_Album", + "variables": [ + { + "ArtistId": 1 + } + ], + "query": { + "fields": { + "albumId": { + "type": "column", + "column": "AlbumId", + "fields": null + }, + "artistId": { + "type": "column", + "column": "ArtistId", + "fields": null + }, + "title": { + "type": "column", + "column": "Title", + "fields": null + } + }, + "predicate": { + "type": "and", + "expressions": [ + { + "type": "and", + "expressions": [ + { + "type": "binary_comparison_operator", + "column": { + "type": "column", + "name": "ArtistId", + "path": [] + }, + "operator": "_eq", + "value": { + "type": "variable", + "name": "ArtistId" + } + } + ] + } + ] + } + }, + "arguments": {}, + "collection_relationships": {} +} \ No newline at end of file diff --git a/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.statement.sql b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.statement.sql new file mode 100644 index 0000000..4c8543a --- /dev/null +++ b/crates/ndc-clickhouse/tests/query_builder/chinook/03_variables/03_single_set.statement.sql @@ -0,0 +1,48 @@ +WITH "_vars" AS ( + SELECT + * + FROM + format( + JSONColumns, + '{"_varset_id":[1],"_var_ArtistId":[1]}' + ) +) +SELECT + toJSONString( + groupArray( + cast( + "_rowset"."_rowset", + 'Tuple(rows Array(Tuple("albumId" Int32, "artistId" Int32, "title" String)))' + ) + ) + ) AS "rowsets" +FROM + "_vars" AS "_vars" + LEFT JOIN ( + SELECT + tuple( + groupArray( + tuple( + "_row"."_field_albumId", + "_row"."_field_artistId", + "_row"."_field_title" + ) + ) + ) AS "_rowset", + "_row"."_varset_id" AS "_varset_id" + FROM + ( + SELECT + "_origin"."AlbumId" AS "_field_albumId", + "_origin"."ArtistId" AS "_field_artistId", + "_origin"."Title" AS "_field_title", + "_vars"."_varset_id" AS "_varset_id" + FROM + "_vars" AS "_vars" + CROSS JOIN "Chinook"."Album" AS "_origin" + WHERE + "_origin"."ArtistId" = "_vars"."_var_ArtistId" + ) AS "_row" + GROUP BY + "_row"."_varset_id" + ) AS "_rowset" ON "_vars"."_varset_id" = "_rowset"."_varset_id" FORMAT TabSeparatedRaw; \ No newline at end of file From 23210339cfac42c67d2bef95dc09f947db8f16a0 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Sat, 17 Aug 2024 09:19:00 -0400 Subject: [PATCH 3/5] update docker image to use latest stable rust --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 1f57156..fba6f68 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # https://github.com/LukeMathWalker/cargo-chef -FROM rust:1.75.0 as chef +FROM rust:1.80.1 as chef RUN cargo install cargo-chef WORKDIR /app From cb7fb76078a971b8be595a26e522fd9ba40c4803 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Sat, 17 Aug 2024 09:19:15 -0400 Subject: [PATCH 4/5] tag docker compose test image as debug --- docker-compose.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/docker-compose.yaml b/docker-compose.yaml index 2ea5d93..58de0f3 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,5 +1,6 @@ services: ndc-clickhouse: + image: ndc-clickhouse-debug build: context: . args: From f10c9d3e835cd4754cd5d647566dcc39abe5aa54 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Sat, 17 Aug 2024 09:19:31 -0400 Subject: [PATCH 5/5] bump version to 1.0.1, add changelog --- CHANGELOG.md | 4 ++++ Cargo.lock | 6 +++--- Cargo.toml | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0149099..77adc3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.0.1] + +- Bug fix: remove erroneous group by and order by clauses in `foreach` queries. Remote relationships should now function as expected. The previous fix was incorrect. + ## [1.0.0] - Stable Release diff --git a/Cargo.lock b/Cargo.lock index 4f2283e..f1a2d10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -334,7 +334,7 @@ checksum = "97af0562545a7d7f3d9222fcf909963bec36dcb502afaacab98c6ffac8da47ce" [[package]] name = "common" -version = "1.0.0" +version = "1.0.1" dependencies = [ "bytes", "peg", @@ -1053,7 +1053,7 @@ dependencies = [ [[package]] name = "ndc-clickhouse" -version = "1.0.0" +version = "1.0.1" dependencies = [ "async-trait", "bytes", @@ -1073,7 +1073,7 @@ dependencies = [ [[package]] name = "ndc-clickhouse-cli" -version = "1.0.0" +version = "1.0.1" dependencies = [ "clap", "common", diff --git a/Cargo.toml b/Cargo.toml index 0f58f2e..e91b15b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,5 +6,5 @@ members = [ ] resolver = "2" -package.version = "1.0.0" +package.version = "1.0.1" package.edition = "2021"