diff --git a/CHANGELOG.md b/CHANGELOG.md index 96d30fe4..926788d7 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 parsing [275](https://github.com/Qrlew/qrlew/pull/275) + ## [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..b6e1a514 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" @@ -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) diff --git a/src/rewriting/mod.rs b/src/rewriting/mod.rs index 8da8c70c..000299a5 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..f02f0b76 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,17 @@ fn last(columns: &Hierarchy) -> Hierarchy { .collect() } +/// 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() + } else { + ident.value.to_lowercase() + } +} + /// A simple SQL query parser with dialect pub fn parse_with_dialect(query: &str, dialect: D) -> Result { @@ -1461,6 +1471,120 @@ mod tests { .map(ToString::to_string); } + + #[test] + 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 + "#; + 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); + } + #[test] fn test_select_all_with_joins() { let mut database = postgresql::test_database();