From d5e10f4a2d03f5c59c3087e855718c88a7243a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=20Anast=C3=A1cio?= Date: Thu, 26 Dec 2024 13:43:15 -0300 Subject: [PATCH 1/2] Fix some clippy warnings --- native/sqlparser_parse/src/datatypes.rs | 78 +++++++++---------------- native/sqlparser_parse/src/lib.rs | 5 +- 2 files changed, 32 insertions(+), 51 deletions(-) diff --git a/native/sqlparser_parse/src/datatypes.rs b/native/sqlparser_parse/src/datatypes.rs index c0ba4ee..fa3c7d7 100644 --- a/native/sqlparser_parse/src/datatypes.rs +++ b/native/sqlparser_parse/src/datatypes.rs @@ -235,7 +235,7 @@ impl From for sqlparser::ast::TableFactor { with_hints: [].to_vec(), }, _ => sqlparser::ast::TableFactor::Table { - name: name, + name, alias: None, args: None, with_hints: [].to_vec(), @@ -244,10 +244,8 @@ impl From for sqlparser::ast::TableFactor { } } impl From for sqlparser::ast::JoinOperator { - fn from(join_operator: JoinOperator) -> Self { - match join_operator { - _ => sqlparser::ast::JoinOperator::CrossJoin, - } + fn from(_: JoinOperator) -> Self { + sqlparser::ast::JoinOperator::CrossJoin } } impl From for sqlparser::ast::Join { @@ -298,7 +296,7 @@ impl From for sqlparser::ast::SetExpr { .map(|l| sqlparser::ast::TableWithJoins::from(l.clone())) .collect(), lateral_views: [].to_vec(), - selection: select.selection.map(|l| sqlparser::ast::Expr::from(l)), + selection: select.selection.map(sqlparser::ast::Expr::from), group_by: select .group_by .iter() @@ -311,7 +309,7 @@ impl From for sqlparser::ast::SetExpr { .iter() .map(|l| sqlparser::ast::Expr::from(l.clone())) .collect(), - having: select.having.map(|l| sqlparser::ast::Expr::from(l)), + having: select.having.map(sqlparser::ast::Expr::from), qualify: None, })) } @@ -339,7 +337,7 @@ impl From for sqlparser::ast::Statement { Statement::Query(query) => { sqlparser::ast::Statement::Query(Box::new(sqlparser::ast::Query { body: Box::new(sqlparser::ast::SetExpr::from(query.body)), - limit: query.limit.map(|l| sqlparser::ast::Expr::from(l)), + limit: query.limit.map(sqlparser::ast::Expr::from), with: None, order_by: query .order_by @@ -432,6 +430,12 @@ impl Wildcard { } } +impl Default for Wildcard { + fn default() -> Self { + Wildcard::new() + } +} + #[derive(NifStruct)] //#[rustler(encode)] #[module = "SqlParser.ExprWithAlias"] @@ -602,7 +606,7 @@ impl TableWithJoins { pub fn new(ast: &sqlparser::ast::TableWithJoins) -> Self { let relation = TableFactor::from(ast.relation.clone()); Self { - relation: relation, + relation, joins: ast .joins .iter() @@ -626,10 +630,7 @@ impl From for Ident { fn from(ident: sqlparser::ast::Ident) -> Self { Self { value: ident.to_string(), - quote_style: match ident.quote_style { - None => None, - Some(style) => Some(style.to_string()), - }, + quote_style: ident.quote_style.map(|s| s.to_string()), } } } @@ -839,10 +840,7 @@ pub enum Value { impl From for Value { fn from(value: sqlparser::ast::Value) -> Self { match value { - sqlparser::ast::Value::Number(num, long) => Self::Number(Number { - value: num, - long: long, - }), + sqlparser::ast::Value::Number(num, long) => Self::Number(Number { value: num, long }), sqlparser::ast::Value::SingleQuotedString(string) => Self::SingleQuotedString(string), sqlparser::ast::Value::DollarQuotedString(_dollar_quoted_string) => { Self::NotImplemented(result_atoms::not_implemented()) @@ -1034,7 +1032,7 @@ impl Expr { value: ExprEnum::InList(InList { expr: Box::new(Expr::new(*expr)), list: list.iter().map(|p| Expr::new(p.clone())).collect(), - negated: negated, + negated, }), }, sqlparser::ast::Expr::InSubquery { @@ -1046,7 +1044,7 @@ impl Expr { value: ExprEnum::InSubquery(InSubquery { expr: Box::new(Expr::new(*expr)), subquery: Box::new(Query::new(*subquery)), - negated: negated, + negated, }), }, sqlparser::ast::Expr::InUnnest { @@ -1058,7 +1056,7 @@ impl Expr { value: ExprEnum::InUnnest(InUnnest { expr: Box::new(Expr::new(*expr)), array_expr: Box::new(Expr::new(*array_expr)), - negated: negated, + negated, }), }, sqlparser::ast::Expr::Between { @@ -1070,7 +1068,7 @@ impl Expr { r#type: type_atoms::in_subquery(), value: ExprEnum::Between(Between { expr: Box::new(Expr::new(*expr)), - negated: negated, + negated, low: Box::new(Expr::new(*low)), high: Box::new(Expr::new(*high)), }), @@ -1092,12 +1090,9 @@ impl Expr { r#type: type_atoms::like(), value: ExprEnum::Like(Like { expr: Box::new(Expr::new(*expr)), - negated: negated, + negated, pattern: Box::new(Expr::new(*pattern)), - escape_char: match escape_char { - Some(c) => Some(c.to_string()), - None => None, - }, + escape_char: escape_char.map(|c| c.to_string()), }), }, sqlparser::ast::Expr::ILike { @@ -1109,12 +1104,9 @@ impl Expr { r#type: type_atoms::ilike(), value: ExprEnum::ILike(ILike { expr: Box::new(Expr::new(*expr)), - negated: negated, + negated, pattern: Box::new(Expr::new(*pattern)), - escape_char: match escape_char { - Some(c) => Some(c.to_string()), - None => None, - }, + escape_char: escape_char.map(|c| c.to_string()), }), }, sqlparser::ast::Expr::SimilarTo { @@ -1126,12 +1118,9 @@ impl Expr { r#type: type_atoms::similar_to(), value: ExprEnum::SimilarTo(SimilarTo { expr: Box::new(Expr::new(*expr)), - negated: negated, + negated, pattern: Box::new(Expr::new(*pattern)), - escape_char: match escape_char { - Some(c) => Some(c.to_string()), - None => None, - }, + escape_char: escape_char.map(|c| c.to_string()), }), }, sqlparser::ast::Expr::AnyOp(expr) => Expr { @@ -1240,11 +1229,8 @@ impl Select { } }) .collect(), - from: ast.from.iter().map(|p| TableWithJoins::new(p)).collect(), - selection: match ast.selection { - Some(expr) => Some(Expr::new(expr)), - None => None, - }, + from: ast.from.iter().map(TableWithJoins::new).collect(), + selection: ast.selection.map(Expr::new), group_by: ast .group_by .iter() @@ -1255,10 +1241,7 @@ impl Select { .iter() .map(|expr| Expr::new(expr.clone())) .collect(), - having: match ast.having { - Some(expr) => Some(Expr::new(expr)), - None => None, - }, + having: ast.having.map(Expr::new), } } } @@ -1352,10 +1335,7 @@ impl Query { nulls_first: order_by_expr.nulls_first, }) .collect(), - limit: match ast.limit { - Some(expr) => Some(Expr::new(expr)), - None => None, - }, + limit: ast.limit.map(Expr::new), offset: match ast.offset { Some(offset) => Some(Offset { value: Expr::new(offset.value), diff --git a/native/sqlparser_parse/src/lib.rs b/native/sqlparser_parse/src/lib.rs index 0c3626a..a610f32 100644 --- a/native/sqlparser_parse/src/lib.rs +++ b/native/sqlparser_parse/src/lib.rs @@ -63,13 +63,14 @@ fn parse_statements( Err(e) => Err(Error::Term(Box::new(e.to_string()))), }; - return ast; + + ast } #[rustler::nif] fn to_sql(ast: Document, _dialect: Dialect) -> Result<(Atom, String), Error> { let statement = sqlparser::ast::Statement::from(ast.statements[0].clone()); - return Ok((atom::ok(), statement.to_string())); + Ok((atom::ok(), statement.to_string())) } rustler::init!("Elixir.SqlParser.Parse"); From dcca13d177b0045ff9f6556d0ccb9038e0245490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=20Anast=C3=A1cio?= Date: Thu, 26 Dec 2024 13:43:37 -0300 Subject: [PATCH 2/2] Remove expr skipped test --- test/sql_parser_test.exs | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/test/sql_parser_test.exs b/test/sql_parser_test.exs index 8a1e7c2..69e9389 100644 --- a/test/sql_parser_test.exs +++ b/test/sql_parser_test.exs @@ -1,8 +1,6 @@ defmodule SqlParserTest do use ExUnit.Case doctest SqlParser - alias SqlParser.Expr - alias SqlParser.Ident # test "to_sql" do # # WHERE b.a = d @@ -26,7 +24,7 @@ defmodule SqlParserTest do %SqlParser.TableWithJoins{ relation: %SqlParser.Table{ name: %SqlParser.ObjectName{ - names: [%Ident{quote_style: nil, value: "a"}] + names: [%SqlParser.Ident{quote_style: nil, value: "a"}] } } } @@ -55,7 +53,7 @@ defmodule SqlParserTest do joins: [], relation: %SqlParser.Table{ name: %SqlParser.ObjectName{ - names: [%Ident{quote_style: nil, value: "a"}] + names: [%SqlParser.Ident{quote_style: nil, value: "a"}] } } } @@ -68,9 +66,9 @@ defmodule SqlParserTest do }, order_by: [ %SqlParser.OrderByExpr{ - expr: %Expr{ + expr: %SqlParser.Expr{ type: :identifier, - value: %Ident{quote_style: nil, value: "f"} + value: %SqlParser.Ident{quote_style: nil, value: "f"} }, asc: nil, nulls_first: nil @@ -119,7 +117,7 @@ defmodule SqlParserTest do assert %SqlParser.Query{ body: %SqlParser.Select{ - selection: %Expr{ + selection: %SqlParser.Expr{ type: :binary_op, value: %SqlParser.BinaryOp{ left: _, @@ -188,21 +186,32 @@ defmodule SqlParserTest do end @exprs %{ - %Expr{ + %SqlParser.Expr{ type: :binary_op, value: %SqlParser.BinaryOp{ - left: %Expr{type: :identifier, value: %Ident{quote_style: nil, value: "a"}}, + left: %SqlParser.Expr{ + type: :identifier, + value: %SqlParser.Ident{quote_style: nil, value: "a"} + }, op: :eq, - right: %Expr{type: :value, value: {:number, "1", false}} + right: %SqlParser.Expr{ + type: :value, + value: %SqlParser.Number{ + long: false, + value: "18446744073709551616.18446744073709551616" + } + } } } => "a = 18446744073709551616.18446744073709551616", - %Expr{ + %SqlParser.Expr{ type: :is_null, - value: %Expr{type: :identifier, value: %Ident{quote_style: nil, value: "c"}} + value: %SqlParser.Expr{ + type: :identifier, + value: %SqlParser.Ident{quote_style: nil, value: "c"} + } } => "c IS NULL" } - @tag :skip test "expr work" do for {expected_selection, expr} <- @exprs do assert {:ok, [query]} = SqlParser.parse("SELECT c as b from a WHERE #{expr}")