From 9a6a29f542c3e0eef1ebd5a1fb1da81b1ffcc9fa Mon Sep 17 00:00:00 2001 From: Gil Mizrahi Date: Fri, 23 Aug 2024 14:10:37 +0300 Subject: [PATCH] scope tests --- .../translation/src/translation/helpers.rs | 32 +++- .../src/translation/query/filtering.rs | 37 +++-- .../configuration.json | 6 + .../request.json | 16 +- .../request.json | 11 +- ...sts__it_select_where_unrelated_exists.snap | 2 +- ...quals_self_nested_object_relationship.snap | 118 +++++--------- ...schema_tests__schema_test__get_schema.snap | 11 +- ...schema_tests__schema_test__get_schema.snap | 11 +- .../src/postgres/query_tests.rs | 6 + ...tests__predicates__filter_using_scope.snap | 67 ++++++++ .../goldenfiles/filter_using_scope.json | 145 ++++++++++++++++++ .../select_where_unrelated_exists.json | 8 +- 13 files changed, 351 insertions(+), 119 deletions(-) create mode 100644 crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__query_tests__predicates__filter_using_scope.snap create mode 100644 crates/tests/tests-common/goldenfiles/filter_using_scope.json diff --git a/crates/query-engine/translation/src/translation/helpers.rs b/crates/query-engine/translation/src/translation/helpers.rs index a8ea997c0..6f4a99dc1 100644 --- a/crates/query-engine/translation/src/translation/helpers.rs +++ b/crates/query-engine/translation/src/translation/helpers.rs @@ -65,18 +65,42 @@ impl CurrentTableAndScope { scope: vec![], } } - pub fn get(&self, i: usize) -> Result<&TableSourceAndReference, Error> { - if i == 0 { + pub fn get(&self, scope_index: usize) -> Result<&TableSourceAndReference, Error> { + if scope_index == 0 { Ok(&self.current_table) } else { self.scope - .get(self.scope.len() - i) + .get(self.scope.len() - scope_index) .ok_or(Error::ScopeOutOfRange { - requested: i, + requested: scope_index, size: self.scope.len() + 1, }) } } + /// Rewind scope to get scope in index. + pub fn get_scope(&self, scope_index: usize) -> Result { + if scope_index == 0 { + Ok(self.clone()) + } else { + let mut scope = self.scope.clone(); + for _ in 1..scope_index { + scope.pop().ok_or(Error::ScopeOutOfRange { + requested: scope_index, + size: self.scope.len() + 1, + })?; + } + + let current_table = scope.pop().ok_or(Error::ScopeOutOfRange { + requested: scope_index, + size: self.scope.len() + 1, + })?; + + Ok(CurrentTableAndScope { + current_table, + scope, + }) + } + } pub fn push(&self, table: TableSourceAndReference) -> Self { let mut scope = self.scope.clone(); scope.push(self.current_table.clone()); diff --git a/crates/query-engine/translation/src/translation/query/filtering.rs b/crates/query-engine/translation/src/translation/query/filtering.rs index 274c977fc..ceae69584 100644 --- a/crates/query-engine/translation/src/translation/query/filtering.rs +++ b/crates/query-engine/translation/src/translation/query/filtering.rs @@ -475,22 +475,29 @@ fn translate_comparison_value( path, scope, } => { - if !path.is_empty() { - todo!("comparison value with path not supported"); - } - if scope.is_some() { - todo!("comparison value with scope not supported"); - } + // get the scope. + let (table_ref, joins) = match *scope { + Some(scope_index) if scope_index > 0 => { + let tables = tables.get_scope(scope_index)?; + translate_comparison_pathelements(env, state, &tables, &path)? + } + _ => translate_comparison_pathelements(env, state, tables, &path)?, + }; - translate_comparison_target( - env, - state, - tables, - &models::ComparisonTarget::Column { - name: name.clone(), - field_path: field_path.clone(), - }, - ) + // get the unrelated table information from the metadata. + let collection_info = env.lookup_fields_info(&table_ref.source)?; + let ColumnInfo { name, .. } = collection_info.lookup_column(name)?; + + Ok(( + wrap_in_field_path( + &field_path.into(), + sql::ast::Expression::ColumnReference(sql::ast::ColumnReference::TableColumn { + table: table_ref.reference.clone(), + name, + }), + ), + joins, + )) } models::ComparisonValue::Scalar { value: json_value } => { Ok((values::translate(env, state, json_value, typ)?, vec![])) diff --git a/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/configuration.json b/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/configuration.json index 5a44a8c80..5c805de8b 100644 --- a/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/configuration.json +++ b/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/configuration.json @@ -197,6 +197,12 @@ "operatorKind": "equal", "argumentType": "varchar", "isInfix": true + }, + "_like": { + "operatorName": "LIKE", + "operatorKind": "custom", + "argumentType": "varchar", + "isInfix": true } }, "typeRepresentation": null diff --git a/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/request.json b/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/request.json index 6f868e20e..f95020762 100644 --- a/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/request.json +++ b/crates/query-engine/translation/tests/goldenfiles/select_where_album_id_equals_self_nested_object_relationship/request.json @@ -75,10 +75,10 @@ "type": "column", "name": "Title" }, - "operator": "_eq", + "operator": "_like", "value": { "type": "scalar", - "value": "The album title (1)" + "value": "%t%" } }, { @@ -97,23 +97,23 @@ "type": "column", "name": "Name" }, - "operator": "_eq", + "operator": "_like", "value": { "type": "scalar", - "value": "The Artist name" + "value": "%o%" } }, { "type": "binary_comparison_operator", "column": { "type": "column", - "name": "ArtistId" + "name": "Name" }, - "operator": "_lte", + "operator": "_eq", "value": { "type": "column", - "name": "AlbumId", - "scope": 1 + "name": "Name", + "scope": 2 } } ] diff --git a/crates/query-engine/translation/tests/goldenfiles/select_where_unrelated_exists/request.json b/crates/query-engine/translation/tests/goldenfiles/select_where_unrelated_exists/request.json index a4260d5e2..21b5ff653 100644 --- a/crates/query-engine/translation/tests/goldenfiles/select_where_unrelated_exists/request.json +++ b/crates/query-engine/translation/tests/goldenfiles/select_where_unrelated_exists/request.json @@ -34,17 +34,14 @@ { "type": "binary_comparison_operator", "column": { - "type": "root_collection_column", - "name": "artist_id" + "type": "column", + "name": "id" }, "operator": "_eq", "value": { "type": "column", - "column": { - "type": "column", - "name": "id", - "path": [] - } + "name": "artist_id", + "scope": 1 } } ] diff --git a/crates/query-engine/translation/tests/snapshots/tests__it_select_where_unrelated_exists.snap b/crates/query-engine/translation/tests/snapshots/tests__it_select_where_unrelated_exists.snap index c82c9d5e7..8e3e47b16 100644 --- a/crates/query-engine/translation/tests/snapshots/tests__it_select_where_unrelated_exists.snap +++ b/crates/query-engine/translation/tests/snapshots/tests__it_select_where_unrelated_exists.snap @@ -29,7 +29,7 @@ FROM ( "%1_artist"."Name" = cast($1 as "pg_catalog"."varchar") ) - AND ("%0_album"."ArtistId" = "%1_artist"."ArtistId") + AND ("%1_artist"."ArtistId" = "%0_album"."ArtistId") ) ) ) AS "%3_rows" diff --git a/crates/query-engine/translation/tests/snapshots/tests__select_where_album_id_equals_self_nested_object_relationship.snap b/crates/query-engine/translation/tests/snapshots/tests__select_where_album_id_equals_self_nested_object_relationship.snap index e6266fd7a..651c50020 100644 --- a/crates/query-engine/translation/tests/snapshots/tests__select_where_album_id_equals_self_nested_object_relationship.snap +++ b/crates/query-engine/translation/tests/snapshots/tests__select_where_album_id_equals_self_nested_object_relationship.snap @@ -3,7 +3,7 @@ source: crates/query-engine/translation/tests/tests.rs expression: result --- SELECT - coalesce(json_agg(row_to_json("%14_universe")), '[]') AS "universe" + coalesce(json_agg(row_to_json("%11_universe")), '[]') AS "universe" FROM ( SELECT @@ -11,7 +11,7 @@ FROM FROM ( SELECT - coalesce(json_agg(row_to_json("%15_rows")), '[]') AS "rows" + coalesce(json_agg(row_to_json("%12_rows")), '[]') AS "rows" FROM ( SELECT @@ -30,17 +30,17 @@ FROM FROM ( SELECT - coalesce(json_agg(row_to_json("%12_rows")), '[]') AS "rows" + coalesce(json_agg(row_to_json("%9_rows")), '[]') AS "rows" FROM ( SELECT - "%7_Album"."Title" AS "album", - "%8_RELATIONSHIP_Artist"."Artist" AS "Artist" + "%4_Album"."Title" AS "album", + "%5_RELATIONSHIP_Artist"."Artist" AS "Artist" FROM - "public"."Album" AS "%7_Album" + "public"."Album" AS "%4_Album" LEFT OUTER JOIN LATERAL ( SELECT - row_to_json("%8_RELATIONSHIP_Artist") AS "Artist" + row_to_json("%5_RELATIONSHIP_Artist") AS "Artist" FROM ( SELECT @@ -48,107 +48,71 @@ FROM FROM ( SELECT - coalesce(json_agg(row_to_json("%10_rows")), '[]') AS "rows" + coalesce(json_agg(row_to_json("%7_rows")), '[]') AS "rows" FROM ( SELECT - "%9_Artist"."Name" AS "artist", - "%9_Artist"."ArtistId" AS "ArtistId" + "%6_Artist"."Name" AS "artist", + "%6_Artist"."ArtistId" AS "ArtistId" FROM - "public"."Artist" AS "%9_Artist" + "public"."Artist" AS "%6_Artist" WHERE - ("%7_Album"."ArtistId" = "%9_Artist"."ArtistId") - ) AS "%10_rows" - ) AS "%10_rows" - ) AS "%8_RELATIONSHIP_Artist" - ) AS "%8_RELATIONSHIP_Artist" ON ('true') + ("%4_Album"."ArtistId" = "%6_Artist"."ArtistId") + ) AS "%7_rows" + ) AS "%7_rows" + ) AS "%5_RELATIONSHIP_Artist" + ) AS "%5_RELATIONSHIP_Artist" ON ('true') WHERE - ("%0_Track"."AlbumId" = "%7_Album"."AlbumId") - ) AS "%12_rows" - ) AS "%12_rows" + ("%0_Track"."AlbumId" = "%4_Album"."AlbumId") + ) AS "%9_rows" + ) AS "%9_rows" ) AS "%1_RELATIONSHIP_Album" ) AS "%1_RELATIONSHIP_Album" ON ('true') WHERE EXISTS ( SELECT - 1 + 1 AS "one" FROM + "public"."Album" AS "%2_Album" + WHERE ( - SELECT - "%2_BOOLEXP_Album".* - FROM - ( - SELECT - * - FROM - "public"."Album" AS "%2_BOOLEXP_Album" - WHERE - ( - ( - "%2_BOOLEXP_Album"."Title" = cast($1 as "pg_catalog"."varchar") - ) - AND ( - "%0_Track"."AlbumId" = "%2_BOOLEXP_Album"."AlbumId" - ) - ) - ) AS "%2_BOOLEXP_Album" - ) AS "%3_BOOLEXP_Album" FULL - OUTER JOIN LATERAL ( - SELECT - "%5_BOOLEXP_Artist".* - FROM + ( ( + "%2_Album"."Title" LIKE cast($1 as "pg_catalog"."varchar") + ) + AND EXISTS ( SELECT - * + 1 AS "one" FROM - "public"."Album" AS "%4_BOOLEXP_Album" + "public"."Artist" AS "%3_Artist" WHERE ( ( - "%4_BOOLEXP_Album"."Title" = cast($2 as "pg_catalog"."varchar") - ) - AND ( - "%0_Track"."AlbumId" = "%4_BOOLEXP_Album"."AlbumId" + ( + "%3_Artist"."Name" LIKE cast($2 as "pg_catalog"."varchar") + ) + AND ("%3_Artist"."Name" = "%0_Track"."Name") ) + AND ("%2_Album"."ArtistId" = "%3_Artist"."ArtistId") ) - ) AS "%4_BOOLEXP_Album" - INNER JOIN LATERAL ( - SELECT - * - FROM - "public"."Artist" AS "%5_BOOLEXP_Artist" - WHERE - ( - ( - "%5_BOOLEXP_Artist"."Name" = cast($3 as "pg_catalog"."varchar") - ) - AND ( - "%4_BOOLEXP_Album"."ArtistId" = "%5_BOOLEXP_Artist"."ArtistId" - ) - ) - ) AS "%5_BOOLEXP_Artist" ON ('true') - ) AS "%6_BOOLEXP_Artist" ON ('true') - WHERE - ( - "%3_BOOLEXP_Album"."AlbumId" > "%6_BOOLEXP_Artist"."ArtistId" + ) + ) + AND ("%0_Track"."AlbumId" = "%2_Album"."AlbumId") ) ) ORDER BY "%0_Track"."TrackId" ASC LIMIT 5 - ) AS "%15_rows" - ) AS "%15_rows" - ) AS "%14_universe"; + ) AS "%12_rows" + ) AS "%12_rows" + ) AS "%11_universe"; { 1: String( - "The album title (1)", + "%t%", ), 2: String( - "The album title (2)", - ), - 3: String( - "The Artist name", + "%o%", ), } diff --git a/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap b/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap index 0ba1d78b5..b3d4d3ab7 100644 --- a/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap +++ b/crates/tests/databases-tests/src/citus/snapshots/databases_tests__citus__schema_tests__schema_test__get_schema.snap @@ -5895,5 +5895,14 @@ expression: result "name": "v1_insert_phone_numbers_response" } } - ] + ], + "capabilities": { + "query": { + "aggregates": { + "filter_by": { + "count_scalar_type": "int4" + } + } + } + } } diff --git a/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap b/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap index 015eb5939..232c15518 100644 --- a/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap +++ b/crates/tests/databases-tests/src/cockroach/snapshots/databases_tests__cockroach__schema_tests__schema_test__get_schema.snap @@ -5232,5 +5232,14 @@ expression: result "name": "v1_insert_pg_extension_spatial_ref_sys_response" } } - ] + ], + "capabilities": { + "query": { + "aggregates": { + "filter_by": { + "count_scalar_type": "int4" + } + } + } + } } diff --git a/crates/tests/databases-tests/src/postgres/query_tests.rs b/crates/tests/databases-tests/src/postgres/query_tests.rs index 190269c1c..ca854dfdd 100644 --- a/crates/tests/databases-tests/src/postgres/query_tests.rs +++ b/crates/tests/databases-tests/src/postgres/query_tests.rs @@ -298,6 +298,12 @@ mod predicates { insta::assert_json_snapshot!(result); } + #[tokio::test] + async fn filter_using_scope() { + let result = run_query(create_router().await, "filter_using_scope").await; + insta::assert_json_snapshot!(result); + } + #[tokio::test] async fn filter_by_nested_field_collection() { let result = run_query(create_router().await, "filter_by_nested_field_collection").await; diff --git a/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__query_tests__predicates__filter_using_scope.snap b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__query_tests__predicates__filter_using_scope.snap new file mode 100644 index 000000000..424d0b631 --- /dev/null +++ b/crates/tests/databases-tests/src/postgres/snapshots/databases_tests__postgres__query_tests__predicates__filter_using_scope.snap @@ -0,0 +1,67 @@ +--- +source: crates/tests/databases-tests/src/postgres/query_tests.rs +expression: result +--- +[ + { + "rows": [ + { + "track": "Body Count", + "AlbumId": 18, + "Album": { + "rows": [ + { + "album": "Body Count", + "Artist": { + "rows": [ + { + "artist": "Body Count", + "ArtistId": 13 + } + ] + } + } + ] + } + }, + { + "track": "Iron Maiden", + "AlbumId": 102, + "Album": { + "rows": [ + { + "album": "Live After Death", + "Artist": { + "rows": [ + { + "artist": "Iron Maiden", + "ArtistId": 90 + } + ] + } + } + ] + } + }, + { + "track": "Iron Maiden", + "AlbumId": 104, + "Album": { + "rows": [ + { + "album": "Live At Donington 1992 (Disc 2)", + "Artist": { + "rows": [ + { + "artist": "Iron Maiden", + "ArtistId": 90 + } + ] + } + } + ] + } + } + ] + } +] diff --git a/crates/tests/tests-common/goldenfiles/filter_using_scope.json b/crates/tests/tests-common/goldenfiles/filter_using_scope.json new file mode 100644 index 000000000..f95020762 --- /dev/null +++ b/crates/tests/tests-common/goldenfiles/filter_using_scope.json @@ -0,0 +1,145 @@ +{ + "collection": "Track", + "query": { + "fields": { + "track": { + "type": "column", + "column": "Name", + "arguments": {} + }, + "AlbumId": { + "type": "column", + "column": "AlbumId", + "arguments": {} + }, + "Album": { + "type": "relationship", + "relationship": "TrackToAlbum", + "arguments": {}, + "query": { + "fields": { + "album": { + "type": "column", + "column": "Title", + "arguments": {} + }, + "Artist": { + "type": "relationship", + "relationship": "AlbumToArtist", + "arguments": {}, + "query": { + "fields": { + "artist": { + "type": "column", + "column": "Name", + "arguments": {} + }, + "ArtistId": { + "type": "column", + "column": "ArtistId", + "arguments": {} + } + } + } + } + } + } + } + }, + "order_by": { + "elements": [ + { + "order_direction": "asc", + "target": { + "type": "column", + "name": "TrackId", + "path": [] + } + } + ] + }, + "limit": 5, + "predicate": { + "type": "exists", + "in_collection": { + "type": "related", + "relationship": "TrackToAlbum", + "arguments": {} + }, + "predicate": { + "type": "and", + "expressions": [ + { + "type": "binary_comparison_operator", + "column": { + "type": "column", + "name": "Title" + }, + "operator": "_like", + "value": { + "type": "scalar", + "value": "%t%" + } + }, + { + "type": "exists", + "in_collection": { + "type": "related", + "relationship": "AlbumToArtist", + "arguments": {} + }, + "predicate": { + "type": "and", + "expressions": [ + { + "type": "binary_comparison_operator", + "column": { + "type": "column", + "name": "Name" + }, + "operator": "_like", + "value": { + "type": "scalar", + "value": "%o%" + } + }, + { + "type": "binary_comparison_operator", + "column": { + "type": "column", + "name": "Name" + }, + "operator": "_eq", + "value": { + "type": "column", + "name": "Name", + "scope": 2 + } + } + ] + } + } + ] + } + } + }, + "arguments": {}, + "collection_relationships": { + "TrackToAlbum": { + "column_mapping": { + "AlbumId": "AlbumId" + }, + "relationship_type": "object", + "target_collection": "Album", + "arguments": {} + }, + "AlbumToArtist": { + "column_mapping": { + "ArtistId": "ArtistId" + }, + "relationship_type": "object", + "target_collection": "Artist", + "arguments": {} + } + } +} diff --git a/crates/tests/tests-common/goldenfiles/select_where_unrelated_exists.json b/crates/tests/tests-common/goldenfiles/select_where_unrelated_exists.json index 8a222f6d8..c92b647d2 100644 --- a/crates/tests/tests-common/goldenfiles/select_where_unrelated_exists.json +++ b/crates/tests/tests-common/goldenfiles/select_where_unrelated_exists.json @@ -33,16 +33,14 @@ { "type": "binary_comparison_operator", "column": { - "type": "root_collection_column", + "type": "column", "name": "ArtistId" }, "operator": "_eq", "value": { "type": "column", - "column": { - "type": "column", - "name": "ArtistId" - } + "name": "ArtistId", + "scope": 1 } } ]