From 1fd4e239e8019ddafa304c256ff1b1eb3618b61a Mon Sep 17 00:00:00 2001 From: Andi Cuko Date: Thu, 28 Mar 2024 15:43:58 +0100 Subject: [PATCH 1/7] ok --- src/rewriting/mod.rs | 1 + src/sql/relation.rs | 92 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/src/rewriting/mod.rs b/src/rewriting/mod.rs index 4b444816..37c130cb 100644 --- a/src/rewriting/mod.rs +++ b/src/rewriting/mod.rs @@ -185,6 +185,7 @@ mod tests { let dp_parameters = DpParameters::from_epsilon_delta(1., 1e-3); let queries = [ + "SELECT city, COUNT(*) FROM order_table o JOIN user_table u ON(o.id=u.id) GROUP BY city ORDER BY city", "SELECT order_id, sum(price) FROM item_table GROUP BY order_id", "SELECT order_id, sum(price), sum(distinct price) FROM item_table GROUP BY order_id HAVING count(*) > 2", "SELECT order_id, sum(order_id) FROM item_table GROUP BY order_id", diff --git a/src/sql/relation.rs b/src/sql/relation.rs index 5422d300..f9bab7f1 100644 --- a/src/sql/relation.rs +++ b/src/sql/relation.rs @@ -342,38 +342,37 @@ impl<'a, T: QueryToRelationTranslator + Copy + Clone> VisitedQueryRelations<'a, let mut named_exprs: Vec<(String, Expr)> = vec![]; - // The select all forces the preservation of names for non ambiguous - // columns. In this vector we collect those names. They are needed - // to update the column mapping for order by limit and offset. + // It stores the update for the column mapping: + // (old name in columns, new name forced by the select) let mut renamed_columns: Vec<(Identifier, Identifier)> = vec![]; for select_item in select_items { match select_item { - ast::SelectItem::UnnamedExpr(expr) => named_exprs.push(( - match expr { - // Pull the original name for implicit aliasing + ast::SelectItem::UnnamedExpr(expr) => { + // Pull the original name for implicit aliasing + let implicit_alias = match expr { ast::Expr::Identifier(ident) => { - if let Some(_) = ident.quote_style { - ident.value.clone() - } else { - ident.value.to_lowercase() - } + lower_case_unquoted_ident(ident) }, ast::Expr::CompoundIdentifier(idents) => { - let iden = idents.last().unwrap(); - if let Some(_) = iden.quote_style { - iden.value.clone() - } else { - iden.value.to_lowercase() - } + let ident = idents.last().unwrap(); + lower_case_unquoted_ident(ident) } expr => namer::name_from_content(FIELD, &expr), - }, - self.translator.try_expr(expr,columns)? - )), + }; + let implicit_alias_ident = Identifier::from_name(implicit_alias.clone()); + if let Some(name) = columns.get(&implicit_alias_ident) { + renamed_columns.push((name.clone(), implicit_alias_ident)); + }; + named_exprs.push((implicit_alias, self.translator.try_expr(expr,columns)?)) + }, ast::SelectItem::ExprWithAlias { expr, alias } => { + let alias_ident = Identifier::from_name(alias.clone().value); + if let Some(name) = columns.get(&alias_ident) { + renamed_columns.push((name.clone(), alias_ident)); + }; named_exprs.push((alias.clone().value, self.translator.try_expr(expr,columns)?)) - } + }, ast::SelectItem::QualifiedWildcard(_, _) => todo!(), ast::SelectItem::Wildcard(_) => { // push all names that are present in the from into named_exprs. @@ -777,6 +776,18 @@ fn last(columns: &Hierarchy) -> Hierarchy { .collect() } +/// If the identifier is quoted we keep its name as it is +/// If it is unquoted we keep the lowercase name during parsing. +/// This allows us to quote any identifier when rendering the relation +/// to a query +fn lower_case_unquoted_ident(ident: &ast::Ident) -> String { + if let Some(_) = ident.quote_style { + ident.value.clone() + } else { + ident.value.to_lowercase() + } +} + /// A simple SQL query parser with dialect pub fn parse_with_dialect(query: &str, dialect: D) -> Result { @@ -1466,6 +1477,45 @@ mod tests { let mut database = postgresql::test_database(); let relations = database.relations(); + let query_str = r#" + SELECT city AS date FROM order_table o JOIN user_table u ON (o.id=u.id) GROUP BY u.city ORDER BY date + "#; + let query = parse(query_str).unwrap(); + let relation = Relation::try_from(QueryWithRelations::new( + &query, + &relations + )) + .unwrap(); + relation.display_dot().unwrap(); + let query: &str = &ast::Query::from(&relation).to_string(); + println!("{query}"); + _ = database + .query(query) + .unwrap() + .iter() + .map(ToString::to_string); + + let query_str = r#" + SELECT * + FROM table_2 AS t1 INNER JOIN table_2 AS t2 USING(x) INNER JOIN table_2 AS t3 USING(x) + WHERE x > 50 + ORDER BY x, t2.y, t2.z + "#; + let query = parse(query_str).unwrap(); + let relation = Relation::try_from(QueryWithRelations::new( + &query, + &relations + )) + .unwrap(); + relation.display_dot().unwrap(); + let query: &str = &ast::Query::from(&relation).to_string(); + println!("{query}"); + _ = database + .query(query) + .unwrap() + .iter() + .map(ToString::to_string); + let query_str = r#" SELECT * FROM table_2 AS t1 INNER JOIN table_2 AS t2 USING(x) INNER JOIN table_2 AS t3 USING(x) From eddcf00cd84aeeeaf9c8e2ca6db0131ba523647d Mon Sep 17 00:00:00 2001 From: Andi Cuko Date: Thu, 28 Mar 2024 16:40:04 +0100 Subject: [PATCH 2/7] tests --- src/sql/relation.rs | 98 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/src/sql/relation.rs b/src/sql/relation.rs index f9bab7f1..932b474f 100644 --- a/src/sql/relation.rs +++ b/src/sql/relation.rs @@ -1472,10 +1472,100 @@ mod tests { .map(ToString::to_string); } + #[test] - fn test_select_all_with_joins() { + fn test_order_by() { let mut database = postgresql::test_database(); let relations = database.relations(); + let query_str = r#" + SELECT * FROM user_table u ORDER BY u.city, u.id + "#; + let query = parse(query_str).unwrap(); + let relation = Relation::try_from(QueryWithRelations::new( + &query, + &relations + )) + .unwrap(); + relation.display_dot().unwrap(); + let query: &str = &ast::Query::from(&relation).to_string(); + println!("{query}"); + _ = database + .query(query) + .unwrap() + .iter() + .map(ToString::to_string); + + let query_str = r#" + SELECT * FROM order_table o JOIN user_table u ON (o.id=u.id) ORDER BY city + "#; + let query = parse(query_str).unwrap(); + let relation = Relation::try_from(QueryWithRelations::new( + &query, + &relations + )) + .unwrap(); + relation.display_dot().unwrap(); + let query: &str = &ast::Query::from(&relation).to_string(); + println!("{query}"); + _ = database + .query(query) + .unwrap() + .iter() + .map(ToString::to_string); + + let query_str = r#" + SELECT * FROM order_table o JOIN user_table u ON (o.id=u.id) ORDER BY o.id + "#; + let query = parse(query_str).unwrap(); + let relation = Relation::try_from(QueryWithRelations::new( + &query, + &relations + )) + .unwrap(); + relation.display_dot().unwrap(); + let query: &str = &ast::Query::from(&relation).to_string(); + println!("{query}"); + _ = database + .query(query) + .unwrap() + .iter() + .map(ToString::to_string); + + let query_str = r#" + SELECT city, SUM(o.id) FROM order_table o JOIN user_table u ON (o.id=u.id) GROUP BY city ORDER BY city + "#; + let query = parse(query_str).unwrap(); + let relation = Relation::try_from(QueryWithRelations::new( + &query, + &relations + )) + .unwrap(); + relation.display_dot().unwrap(); + let query: &str = &ast::Query::from(&relation).to_string(); + println!("{query}"); + _ = database + .query(query) + .unwrap() + .iter() + .map(ToString::to_string); + + let query_str = r#" + SELECT city AS mycity, SUM(o.id) AS mysum FROM order_table o JOIN user_table u ON (o.id=u.id) GROUP BY mycity ORDER BY mycity, mysum + "#; + let query = parse(query_str).unwrap(); + let relation = Relation::try_from(QueryWithRelations::new( + &query, + &relations + )) + .unwrap(); + relation.display_dot().unwrap(); + let query: &str = &ast::Query::from(&relation).to_string(); + println!("{query}"); + _ = database + .query(query) + .unwrap() + .iter() + .map(ToString::to_string); let query_str = r#" SELECT city AS date FROM order_table o JOIN user_table u ON (o.id=u.id) GROUP BY u.city ORDER BY date @@ -1494,6 +1584,12 @@ mod tests { .unwrap() .iter() .map(ToString::to_string); + } + + #[test] + fn test_select_all_with_joins() { + let mut database = postgresql::test_database(); + let relations = database.relations(); let query_str = r#" SELECT * From 4ea3d8f2943d58152d1c413bb4dfa3b32a4ef03a Mon Sep 17 00:00:00 2001 From: Andi Cuko Date: Thu, 28 Mar 2024 16:41:38 +0100 Subject: [PATCH 3/7] changelog --- CHANGELOG.md | 5 +++++ Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96d30fe4..98cc9d10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] + +## [0.9.17] - 2024-03-28 +### Fixed +- Fix order by + ## [0.9.16] - 2024-03-20 ### Changed - Expr::divide makes sure we have non-zeros in the denominator [273](https://github.com/Qrlew/qrlew/pull/273) diff --git a/Cargo.toml b/Cargo.toml index f855223d..25373468 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] authors = ["Nicolas Grislain "] name = "qrlew" -version = "0.9.16" +version = "0.9.17" edition = "2021" description = "Sarus Qrlew Engine" documentation = "https://docs.rs/qrlew" From 23997f3551245ef5a27cb2860b18d4e6c6cb43a6 Mon Sep 17 00:00:00 2001 From: Andi Cuko Date: Thu, 28 Mar 2024 16:43:52 +0100 Subject: [PATCH 4/7] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98cc9d10..926788d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.9.17] - 2024-03-28 ### Fixed -- Fix order by +- Fix order by parsing [275](https://github.com/Qrlew/qrlew/pull/275) ## [0.9.16] - 2024-03-20 ### Changed From 2106d2abb65b7af678812aa2b1f1503754b95268 Mon Sep 17 00:00:00 2001 From: Andi Cuko Date: Thu, 28 Mar 2024 16:52:59 +0100 Subject: [PATCH 5/7] clean --- src/sql/relation.rs | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/src/sql/relation.rs b/src/sql/relation.rs index 932b474f..f02f0b76 100644 --- a/src/sql/relation.rs +++ b/src/sql/relation.rs @@ -776,10 +776,9 @@ fn last(columns: &Hierarchy) -> Hierarchy { .collect() } -/// If the identifier is quoted we keep its name as it is -/// If it is unquoted we keep the lowercase name during parsing. -/// This allows us to quote any identifier when rendering the relation -/// to a query +/// Returns the identifier value. If it is quoted it returns its value +/// as it is whereas if unquoted it returns the lowercase value. +/// Used to create relations field's name. fn lower_case_unquoted_ident(ident: &ast::Ident) -> String { if let Some(_) = ident.quote_style { ident.value.clone() @@ -1591,27 +1590,6 @@ mod tests { let mut database = postgresql::test_database(); let relations = database.relations(); - let query_str = r#" - SELECT * - FROM table_2 AS t1 INNER JOIN table_2 AS t2 USING(x) INNER JOIN table_2 AS t3 USING(x) - WHERE x > 50 - ORDER BY x, t2.y, t2.z - "#; - let query = parse(query_str).unwrap(); - let relation = Relation::try_from(QueryWithRelations::new( - &query, - &relations - )) - .unwrap(); - relation.display_dot().unwrap(); - let query: &str = &ast::Query::from(&relation).to_string(); - println!("{query}"); - _ = database - .query(query) - .unwrap() - .iter() - .map(ToString::to_string); - let query_str = r#" SELECT * FROM table_2 AS t1 INNER JOIN table_2 AS t2 USING(x) INNER JOIN table_2 AS t3 USING(x) From 8a39e99f17ac10e701438032f8a0377077a6bb27 Mon Sep 17 00:00:00 2001 From: Andi Cuko Date: Thu, 28 Mar 2024 17:13:53 +0100 Subject: [PATCH 6/7] fix version of yup-oauth2, dependency of gcp-bigquery-client --- Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 25373468..f831beb9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,12 +40,13 @@ tokio = { version = "1", features = ["full"], optional = true } gcp-bigquery-client = { version = "0.18", optional = true } wiremock = { version = "0.5.19", optional = true } tempfile = { version = "3.6.0", optional = true } +yup-oauth2 = { version = "^8.3.2", optional = true } # 8.3.3 makes the compiling of gcp-bigquery-client fail [features] # Use SQLite for tests and examples sqlite = ["dep:rusqlite"] mssql = ["dep:sqlx", "dep:tokio"] -bigquery = ["dep:gcp-bigquery-client", "dep:wiremock", "dep:tempfile"] +bigquery = ["dep:gcp-bigquery-client", "dep:wiremock", "dep:tempfile", "dep:yup-oauth2"] # Tests checked_injections = [] # Multiplicity features are tested on large datasets (may take too much memory) From b7bd12bc0c3dfb4b4977ea2b756c113acd1c0f52 Mon Sep 17 00:00:00 2001 From: Andi Cuko Date: Thu, 28 Mar 2024 17:19:19 +0100 Subject: [PATCH 7/7] fix with = --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f831beb9..b6e1a514 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ tokio = { version = "1", features = ["full"], optional = true } gcp-bigquery-client = { version = "0.18", optional = true } wiremock = { version = "0.5.19", optional = true } tempfile = { version = "3.6.0", optional = true } -yup-oauth2 = { version = "^8.3.2", optional = true } # 8.3.3 makes the compiling of gcp-bigquery-client fail +yup-oauth2 = { version = "=8.3.2", optional = true } # 8.3.3 makes the compiling of gcp-bigquery-client fail [features] # Use SQLite for tests and examples