From d58bbb8f9f0dac4ca79bf4074efb9c8bea475e38 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Thu, 2 May 2019 20:41:28 +0300 Subject: [PATCH 01/11] Update doc comments (The `SQLBetween` change is to fix a `cargo doc` warning.) --- src/sqlast/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/sqlast/mod.rs b/src/sqlast/mod.rs index 0a5bb6ba..81668b2f 100644 --- a/src/sqlast/mod.rs +++ b/src/sqlast/mod.rs @@ -41,8 +41,11 @@ fn comma_separated_string(vec: &[T]) -> String { /// Identifier name, in the originally quoted form (e.g. `"id"`) pub type SQLIdent = String; -/// Represents a parsed SQL expression, which is a common building -/// block of SQL statements (the part after SELECT, WHERE, etc.) +/// An SQL expression of any type. +/// +/// The parser does not distinguish between expressions of different types +/// (e.g. boolean vs string), so the caller must handle expressions of +/// inappropriate type, like `WHERE 1` or `SELECT 1=1`, as necessary. #[derive(Debug, Clone, PartialEq)] pub enum ASTNode { /// Identifier e.g. table name or column name @@ -72,7 +75,7 @@ pub enum ASTNode { subquery: Box, negated: bool, }, - /// [ NOT ] BETWEEN AND + /// ` [ NOT ] BETWEEN AND ` SQLBetween { expr: Box, negated: bool, From fe635350f07d578aa3b86adbe87d636575ea3cad Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Thu, 2 May 2019 20:05:12 +0300 Subject: [PATCH 02/11] Improve INSERT tests De-duplicate and check for specific error in `parse_insert_invalid`. --- tests/sqlparser_postgres.rs | 142 +++++++++++------------------------- 1 file changed, 41 insertions(+), 101 deletions(-) diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 98cdb902..b4d0ff74 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -21,77 +21,44 @@ fn test_prev_index() { } #[test] -fn parse_simple_insert() { - let sql = String::from("INSERT INTO customer VALUES(1, 2, 3)"); - match verified_stmt(&sql) { - SQLStatement::SQLInsert { - table_name, - columns, - values, - .. - } => { - assert_eq!(table_name.to_string(), "customer"); - assert!(columns.is_empty()); - assert_eq!( - vec![vec![ - ASTNode::SQLValue(Value::Long(1)), - ASTNode::SQLValue(Value::Long(2)), - ASTNode::SQLValue(Value::Long(3)) - ]], - values - ); - } - _ => unreachable!(), - } -} +fn parse_insert_values() { + let sql = "INSERT INTO customer VALUES(1, 2, 3)"; + check_one(sql, "customer", vec![]); -#[test] -fn parse_common_insert() { - let sql = String::from("INSERT INTO public.customer VALUES(1, 2, 3)"); - match verified_stmt(&sql) { - SQLStatement::SQLInsert { - table_name, - columns, - values, - .. - } => { - assert_eq!(table_name.to_string(), "public.customer"); - assert!(columns.is_empty()); - assert_eq!( - vec![vec![ - ASTNode::SQLValue(Value::Long(1)), - ASTNode::SQLValue(Value::Long(2)), - ASTNode::SQLValue(Value::Long(3)) - ]], - values - ); - } - _ => unreachable!(), - } -} + let sql = "INSERT INTO public.customer VALUES(1, 2, 3)"; + check_one(sql, "public.customer", vec![]); -#[test] -fn parse_complex_insert() { - let sql = String::from("INSERT INTO db.public.customer VALUES(1, 2, 3)"); - match verified_stmt(&sql) { - SQLStatement::SQLInsert { - table_name, - columns, - values, - .. - } => { - assert_eq!(table_name.to_string(), "db.public.customer"); - assert!(columns.is_empty()); - assert_eq!( - vec![vec![ - ASTNode::SQLValue(Value::Long(1)), - ASTNode::SQLValue(Value::Long(2)), - ASTNode::SQLValue(Value::Long(3)) - ]], - values - ); + let sql = "INSERT INTO db.public.customer VALUES(1, 2, 3)"; + check_one(sql, "db.public.customer", vec![]); + + let sql = "INSERT INTO public.customer (id, name, active) VALUES(1, 2, 3)"; + check_one( + sql, + "public.customer", + vec!["id".to_string(), "name".to_string(), "active".to_string()], + ); + + fn check_one(sql: &str, expected_table_name: &str, expected_columns: Vec) { + match verified_stmt(sql) { + SQLStatement::SQLInsert { + table_name, + columns, + values, + .. + } => { + assert_eq!(table_name.to_string(), expected_table_name); + assert_eq!(columns, expected_columns); + assert_eq!( + vec![vec![ + ASTNode::SQLValue(Value::Long(1)), + ASTNode::SQLValue(Value::Long(2)), + ASTNode::SQLValue(Value::Long(3)) + ]], + values + ); + } + _ => unreachable!(), } - _ => unreachable!(), } } @@ -109,41 +76,14 @@ fn parse_no_table_name() { assert!(ast.is_err()); } -#[test] -fn parse_insert_with_columns() { - let sql = String::from("INSERT INTO public.customer (id, name, active) VALUES(1, 2, 3)"); - match verified_stmt(&sql) { - SQLStatement::SQLInsert { - table_name, - columns, - values, - .. - } => { - assert_eq!(table_name.to_string(), "public.customer"); - assert_eq!( - columns, - vec!["id".to_string(), "name".to_string(), "active".to_string()] - ); - assert_eq!( - vec![vec![ - ASTNode::SQLValue(Value::Long(1)), - ASTNode::SQLValue(Value::Long(2)), - ASTNode::SQLValue(Value::Long(3)) - ]], - values - ); - } - _ => unreachable!(), - } -} - #[test] fn parse_insert_invalid() { - let sql = String::from("INSERT public.customer (id, name, active) VALUES (1, 2, 3)"); - match Parser::parse_sql(&PostgreSqlDialect {}, sql) { - Err(_) => {} - _ => unreachable!(), - } + let sql = "INSERT public.customer (id, name, active) VALUES (1, 2, 3)"; + let res = parse_sql_statements(sql); + assert_eq!( + ParserError::ParserError("Expected INTO, found: public".to_string()), + res.unwrap_err() + ); } #[test] From 0233604f9beade7476018ff64af1492ce078bc1d Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Fri, 3 May 2019 00:01:47 +0300 Subject: [PATCH 03/11] Remove extraneous tests `parse_example_value` parses as compound identifier, which makes no sense ("SARAH"."LEWISE@sakilacustomer"."org") `parse_function_now` is unnecessary since we already test the parsing of functions in `parse_scalar_function_in_projection` --- tests/sqlparser_postgres.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index b4d0ff74..047d6ce9 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -310,20 +310,6 @@ fn parse_timestamps_with_millis_example() { //assert_eq!(sql, ast.to_string()); } -#[test] -fn parse_example_value() { - let sql = "SARAH.LEWIS@sakilacustomer.org"; - let ast = parse_sql_expr(sql); - assert_eq!(sql, ast.to_string()); -} - -#[test] -fn parse_function_now() { - let sql = "now()"; - let ast = parse_sql_expr(sql); - assert_eq!(sql, ast.to_string()); -} - fn verified_stmt(query: &str) -> SQLStatement { one_statement_parses_to(query, query) } From d1b088bd4382aa803dde6016056f2e7c27811769 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Fri, 3 May 2019 01:00:08 +0300 Subject: [PATCH 04/11] Switch remaining tests to the standard format --- tests/sqlparser_generic.rs | 30 +++++++++---------- tests/sqlparser_postgres.rs | 57 ++++++++++++++++--------------------- 2 files changed, 37 insertions(+), 50 deletions(-) diff --git a/tests/sqlparser_generic.rs b/tests/sqlparser_generic.rs index e1e96972..9e00d91b 100644 --- a/tests/sqlparser_generic.rs +++ b/tests/sqlparser_generic.rs @@ -452,14 +452,12 @@ fn parse_cast() { #[test] fn parse_create_table() { - let sql = String::from( - "CREATE TABLE uk_cities (\ - name VARCHAR(100) NOT NULL,\ - lat DOUBLE NULL,\ - lng DOUBLE NULL)", - ); + let sql = "CREATE TABLE uk_cities (\ + name VARCHAR(100) NOT NULL,\ + lat DOUBLE NULL,\ + lng DOUBLE NULL)"; let ast = one_statement_parses_to( - &sql, + sql, "CREATE TABLE uk_cities (\ name character varying(100) NOT NULL, \ lat double, \ @@ -497,15 +495,13 @@ fn parse_create_table() { #[test] fn parse_create_external_table() { - let sql = String::from( - "CREATE EXTERNAL TABLE uk_cities (\ - name VARCHAR(100) NOT NULL,\ - lat DOUBLE NULL,\ - lng DOUBLE NULL)\ - STORED AS TEXTFILE LOCATION '/tmp/example.csv", - ); + let sql = "CREATE EXTERNAL TABLE uk_cities (\ + name VARCHAR(100) NOT NULL,\ + lat DOUBLE NULL,\ + lng DOUBLE NULL)\ + STORED AS TEXTFILE LOCATION '/tmp/example.csv"; let ast = one_statement_parses_to( - &sql, + sql, "CREATE EXTERNAL TABLE uk_cities (\ name character varying(100) NOT NULL, \ lat double, \ @@ -552,8 +548,8 @@ fn parse_scalar_function_in_projection() { let select = verified_only_select(sql); assert_eq!( &ASTNode::SQLFunction { - name: SQLObjectName(vec![String::from("sqrt")]), - args: vec![ASTNode::SQLIdentifier(String::from("id"))], + name: SQLObjectName(vec!["sqrt".to_string()]), + args: vec![ASTNode::SQLIdentifier("id".to_string())], over: None, }, expr_from_projection(only(&select.projection)) diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 047d6ce9..d3a586a4 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9,7 +9,7 @@ use sqlparser::sqltokenizer::*; #[test] fn test_prev_index() { - let sql: &str = "SELECT version()"; + let sql = "SELECT version()"; let mut parser = parser(sql); assert_eq!(parser.prev_token(), None); assert_eq!(parser.next_token(), Some(Token::make_keyword("SELECT"))); @@ -88,8 +88,7 @@ fn parse_insert_invalid() { #[test] fn parse_create_table_with_defaults() { - let sql = String::from( - "CREATE TABLE public.customer ( + let sql = "CREATE TABLE public.customer ( customer_id integer DEFAULT nextval(public.customer_customer_id_seq) NOT NULL, store_id smallint NOT NULL, first_name character varying(45) NOT NULL, @@ -99,9 +98,8 @@ fn parse_create_table_with_defaults() { activebool boolean DEFAULT true NOT NULL, create_date date DEFAULT now()::text NOT NULL, last_update timestamp without time zone DEFAULT now() NOT NULL, - active integer NOT NULL)", - ); - match one_statement_parses_to(&sql, "") { + active integer NOT NULL)"; + match one_statement_parses_to(sql, "") { SQLStatement::SQLCreateTable { name, columns, @@ -133,8 +131,7 @@ fn parse_create_table_with_defaults() { #[test] fn parse_create_table_from_pg_dump() { - let sql = String::from(" - CREATE TABLE public.customer ( + let sql = "CREATE TABLE public.customer ( customer_id integer DEFAULT nextval('public.customer_customer_id_seq'::regclass) NOT NULL, store_id smallint NOT NULL, first_name character varying(45) NOT NULL, @@ -147,8 +144,8 @@ fn parse_create_table_from_pg_dump() { last_update timestamp without time zone DEFAULT now(), release_year public.year, active integer - )"); - match one_statement_parses_to(&sql, "") { + )"; + match one_statement_parses_to(sql, "") { SQLStatement::SQLCreateTable { name, columns, @@ -202,16 +199,14 @@ fn parse_create_table_from_pg_dump() { #[test] fn parse_create_table_with_inherit() { - let sql = String::from( - "\ - CREATE TABLE bazaar.settings (\ - settings_id uuid PRIMARY KEY DEFAULT uuid_generate_v4() NOT NULL, \ - user_id uuid UNIQUE, \ - value text[], \ - use_metric boolean DEFAULT true\ - )", - ); - match verified_stmt(&sql) { + let sql = "\ + CREATE TABLE bazaar.settings (\ + settings_id uuid PRIMARY KEY DEFAULT uuid_generate_v4() NOT NULL, \ + user_id uuid UNIQUE, \ + value text[], \ + use_metric boolean DEFAULT true\ + )"; + match verified_stmt(sql) { SQLStatement::SQLCreateTable { name, columns, @@ -241,12 +236,9 @@ fn parse_create_table_with_inherit() { #[test] fn parse_alter_table_constraint_primary_key() { - let sql = String::from( - "\ - ALTER TABLE bazaar.address \ - ADD CONSTRAINT address_pkey PRIMARY KEY (address_id)", - ); - match verified_stmt(&sql) { + let sql = "ALTER TABLE bazaar.address \ + ADD CONSTRAINT address_pkey PRIMARY KEY (address_id)"; + match verified_stmt(sql) { SQLStatement::SQLAlterTable { name, .. } => { assert_eq!(name.to_string(), "bazaar.address"); } @@ -256,10 +248,9 @@ fn parse_alter_table_constraint_primary_key() { #[test] fn parse_alter_table_constraint_foreign_key() { - let sql = String::from("\ - ALTER TABLE public.customer \ - ADD CONSTRAINT customer_address_id_fkey FOREIGN KEY (address_id) REFERENCES public.address(address_id)"); - match verified_stmt(&sql) { + let sql = "ALTER TABLE public.customer \ + ADD CONSTRAINT customer_address_id_fkey FOREIGN KEY (address_id) REFERENCES public.address(address_id)"; + match verified_stmt(sql) { SQLStatement::SQLAlterTable { name, .. } => { assert_eq!(name.to_string(), "public.customer"); } @@ -269,7 +260,7 @@ fn parse_alter_table_constraint_foreign_key() { #[test] fn parse_copy_example() { - let sql = String::from(r#"COPY public.actor (actor_id, first_name, last_name, last_update, value) FROM stdin; + let sql = r#"COPY public.actor (actor_id, first_name, last_name, last_update, value) FROM stdin; 1 PENELOPE GUINESS 2006-02-15 09:34:33 0.11111 2 NICK WAHLBERG 2006-02-15 09:34:33 0.22222 3 ED CHASE 2006-02-15 09:34:33 0.312323 @@ -288,8 +279,8 @@ Kwara & Kogi 'awe':5 'awe-inspir':4 'barbarella':1 'cat':13 'conquer':16 'dog':18 'feminist':10 'inspir':6 'monasteri':21 'must':15 'stori':7 'streetcar':2 PHP ₱ USD $ \N Some other value -\\."#); - let ast = one_statement_parses_to(&sql, ""); +\\."#; + let ast = one_statement_parses_to(sql, ""); println!("{:#?}", ast); //assert_eq!(sql, ast.to_string()); } From 9297ffbe1875077bf6cc1d0c571d824549546798 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Fri, 3 May 2019 01:04:44 +0300 Subject: [PATCH 05/11] Move tests using standard SQL from the postgresql-specific file --- tests/sqlparser_generic.rs | 76 +++++++++++++++++++++++++++++++++++++ tests/sqlparser_postgres.rs | 76 ------------------------------------- 2 files changed, 76 insertions(+), 76 deletions(-) diff --git a/tests/sqlparser_generic.rs b/tests/sqlparser_generic.rs index 9e00d91b..633b5c1c 100644 --- a/tests/sqlparser_generic.rs +++ b/tests/sqlparser_generic.rs @@ -7,6 +7,58 @@ use sqlparser::sqlast::*; use sqlparser::sqlparser::*; use sqlparser::sqltokenizer::*; +#[test] +fn parse_insert_values() { + let sql = "INSERT INTO customer VALUES(1, 2, 3)"; + check_one(sql, "customer", vec![]); + + let sql = "INSERT INTO public.customer VALUES(1, 2, 3)"; + check_one(sql, "public.customer", vec![]); + + let sql = "INSERT INTO db.public.customer VALUES(1, 2, 3)"; + check_one(sql, "db.public.customer", vec![]); + + let sql = "INSERT INTO public.customer (id, name, active) VALUES(1, 2, 3)"; + check_one( + sql, + "public.customer", + vec!["id".to_string(), "name".to_string(), "active".to_string()], + ); + + fn check_one(sql: &str, expected_table_name: &str, expected_columns: Vec) { + match verified_stmt(sql) { + SQLStatement::SQLInsert { + table_name, + columns, + values, + .. + } => { + assert_eq!(table_name.to_string(), expected_table_name); + assert_eq!(columns, expected_columns); + assert_eq!( + vec![vec![ + ASTNode::SQLValue(Value::Long(1)), + ASTNode::SQLValue(Value::Long(2)), + ASTNode::SQLValue(Value::Long(3)) + ]], + values + ); + } + _ => unreachable!(), + } + } +} + +#[test] +fn parse_insert_invalid() { + let sql = "INSERT public.customer (id, name, active) VALUES (1, 2, 3)"; + let res = parse_sql_statements(sql); + assert_eq!( + ParserError::ParserError("Expected INTO, found: public".to_string()), + res.unwrap_err() + ); +} + #[test] fn parse_delete_statement() { let sql = "DELETE FROM \"table\""; @@ -542,6 +594,30 @@ fn parse_create_external_table() { } } +#[test] +fn parse_alter_table_constraint_primary_key() { + let sql = "ALTER TABLE bazaar.address \ + ADD CONSTRAINT address_pkey PRIMARY KEY (address_id)"; + match verified_stmt(sql) { + SQLStatement::SQLAlterTable { name, .. } => { + assert_eq!(name.to_string(), "bazaar.address"); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_alter_table_constraint_foreign_key() { + let sql = "ALTER TABLE public.customer \ + ADD CONSTRAINT customer_address_id_fkey FOREIGN KEY (address_id) REFERENCES public.address(address_id)"; + match verified_stmt(sql) { + SQLStatement::SQLAlterTable { name, .. } => { + assert_eq!(name.to_string(), "public.customer"); + } + _ => unreachable!(), + } +} + #[test] fn parse_scalar_function_in_projection() { let sql = "SELECT sqrt(id) FROM foo"; diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index d3a586a4..e9354020 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -20,48 +20,6 @@ fn test_prev_index() { assert_eq!(parser.prev_token(), None); } -#[test] -fn parse_insert_values() { - let sql = "INSERT INTO customer VALUES(1, 2, 3)"; - check_one(sql, "customer", vec![]); - - let sql = "INSERT INTO public.customer VALUES(1, 2, 3)"; - check_one(sql, "public.customer", vec![]); - - let sql = "INSERT INTO db.public.customer VALUES(1, 2, 3)"; - check_one(sql, "db.public.customer", vec![]); - - let sql = "INSERT INTO public.customer (id, name, active) VALUES(1, 2, 3)"; - check_one( - sql, - "public.customer", - vec!["id".to_string(), "name".to_string(), "active".to_string()], - ); - - fn check_one(sql: &str, expected_table_name: &str, expected_columns: Vec) { - match verified_stmt(sql) { - SQLStatement::SQLInsert { - table_name, - columns, - values, - .. - } => { - assert_eq!(table_name.to_string(), expected_table_name); - assert_eq!(columns, expected_columns); - assert_eq!( - vec![vec![ - ASTNode::SQLValue(Value::Long(1)), - ASTNode::SQLValue(Value::Long(2)), - ASTNode::SQLValue(Value::Long(3)) - ]], - values - ); - } - _ => unreachable!(), - } - } -} - #[test] fn parse_invalid_table_name() { let mut parser = parser("db.public..customer"); @@ -76,16 +34,6 @@ fn parse_no_table_name() { assert!(ast.is_err()); } -#[test] -fn parse_insert_invalid() { - let sql = "INSERT public.customer (id, name, active) VALUES (1, 2, 3)"; - let res = parse_sql_statements(sql); - assert_eq!( - ParserError::ParserError("Expected INTO, found: public".to_string()), - res.unwrap_err() - ); -} - #[test] fn parse_create_table_with_defaults() { let sql = "CREATE TABLE public.customer ( @@ -234,30 +182,6 @@ fn parse_create_table_with_inherit() { } } -#[test] -fn parse_alter_table_constraint_primary_key() { - let sql = "ALTER TABLE bazaar.address \ - ADD CONSTRAINT address_pkey PRIMARY KEY (address_id)"; - match verified_stmt(sql) { - SQLStatement::SQLAlterTable { name, .. } => { - assert_eq!(name.to_string(), "bazaar.address"); - } - _ => unreachable!(), - } -} - -#[test] -fn parse_alter_table_constraint_foreign_key() { - let sql = "ALTER TABLE public.customer \ - ADD CONSTRAINT customer_address_id_fkey FOREIGN KEY (address_id) REFERENCES public.address(address_id)"; - match verified_stmt(sql) { - SQLStatement::SQLAlterTable { name, .. } => { - assert_eq!(name.to_string(), "public.customer"); - } - _ => unreachable!(), - } -} - #[test] fn parse_copy_example() { let sql = r#"COPY public.actor (actor_id, first_name, last_name, last_update, value) FROM stdin; From de177f107cd0f879aba60a5893778a71e6602452 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Fri, 3 May 2019 01:10:07 +0300 Subject: [PATCH 06/11] Remove dead datetime-related code 1) Removed unused date/time parsing methods from `Parser` I don't see how the token-based parsing code would ever be used: the date/time literals are usually quoted, like `DATE 'yyyy-mm-dd'` or simply `'YYYYMMDD'`, so the date will be a single token. 2) Removed unused date/time related variants from `Value` and the dependency on `chrono`. We don't support parsing date/time literals at the moment and when we do I think we should store the exact String to let the consumer parse it as they see fit. 3) Removed `parse_timestamps_example` and `parse_timestamps_with_millis_example` tests. They parsed as `number(2016) minus number(02) minus number(15) ` (leaving the time part unparsed) as it makes no sense to try parsing a yyyy-mm-dd value as an SQL expression. --- Cargo.toml | 1 - src/sqlast/value.rs | 14 ---- src/sqlparser.rs | 131 ++++++------------------------------ tests/sqlparser_postgres.rs | 22 ------ 4 files changed, 21 insertions(+), 147 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b24348b6..15376f2c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,6 @@ path = "src/lib.rs" [dependencies] log = "0.4.5" -chrono = "0.4.6" [dev-dependencies] simple_logger = "1.0.1" diff --git a/src/sqlast/value.rs b/src/sqlast/value.rs index a36f8d27..1ca4b62b 100644 --- a/src/sqlast/value.rs +++ b/src/sqlast/value.rs @@ -1,5 +1,3 @@ -use chrono::{offset::FixedOffset, DateTime, NaiveDate, NaiveDateTime, NaiveTime}; - /// SQL values such as int, double, string, timestamp #[derive(Debug, Clone, PartialEq)] pub enum Value { @@ -13,14 +11,6 @@ pub enum Value { NationalStringLiteral(String), /// Boolean value true or false, Boolean(bool), - /// Date value - Date(NaiveDate), - // Time - Time(NaiveTime), - /// Date and time - DateTime(NaiveDateTime), - /// Timstamp with time zone - Timestamp(DateTime), /// NULL value in insert statements, Null, } @@ -33,10 +23,6 @@ impl ToString for Value { Value::SingleQuotedString(v) => format!("'{}'", escape_single_quote_string(v)), Value::NationalStringLiteral(v) => format!("N'{}'", v), Value::Boolean(v) => v.to_string(), - Value::Date(v) => v.to_string(), - Value::Time(v) => v.to_string(), - Value::DateTime(v) => v.to_string(), - Value::Timestamp(v) => format!("{}", v), Value::Null => "NULL".to_string(), } } diff --git a/src/sqlparser.rs b/src/sqlparser.rs index 3b6f9c28..b268040a 100644 --- a/src/sqlparser.rs +++ b/src/sqlparser.rs @@ -20,7 +20,6 @@ use super::dialect::keywords; use super::dialect::Dialect; use super::sqlast::*; use super::sqltokenizer::*; -use chrono::{offset::FixedOffset, DateTime, NaiveDate, NaiveDateTime, NaiveTime}; #[derive(Debug, Clone, PartialEq)] pub enum ParserError { @@ -922,37 +921,29 @@ impl Parser { /// Parse a literal value (numbers, strings, date/time, booleans) fn parse_value(&mut self) -> Result { match self.next_token() { - Some(t) => { - match t { - Token::SQLWord(k) => match k.keyword.as_ref() { - "TRUE" => Ok(Value::Boolean(true)), - "FALSE" => Ok(Value::Boolean(false)), - "NULL" => Ok(Value::Null), - _ => { - return parser_err!(format!( - "No value parser for keyword {}", - k.keyword - )); - } - }, - //TODO: parse the timestamp here (see parse_timestamp_value()) - Token::Number(ref n) if n.contains('.') => match n.parse::() { - Ok(n) => Ok(Value::Double(n)), - Err(e) => parser_err!(format!("Could not parse '{}' as f64: {}", n, e)), - }, - Token::Number(ref n) => match n.parse::() { - Ok(n) => Ok(Value::Long(n)), - Err(e) => parser_err!(format!("Could not parse '{}' as i64: {}", n, e)), - }, - Token::SingleQuotedString(ref s) => { - Ok(Value::SingleQuotedString(s.to_string())) + Some(t) => match t { + Token::SQLWord(k) => match k.keyword.as_ref() { + "TRUE" => Ok(Value::Boolean(true)), + "FALSE" => Ok(Value::Boolean(false)), + "NULL" => Ok(Value::Null), + _ => { + return parser_err!(format!("No value parser for keyword {}", k.keyword)); } - Token::NationalStringLiteral(ref s) => { - Ok(Value::NationalStringLiteral(s.to_string())) - } - _ => parser_err!(format!("Unsupported value: {:?}", t)), + }, + Token::Number(ref n) if n.contains('.') => match n.parse::() { + Ok(n) => Ok(Value::Double(n)), + Err(e) => parser_err!(format!("Could not parse '{}' as f64: {}", n, e)), + }, + Token::Number(ref n) => match n.parse::() { + Ok(n) => Ok(Value::Long(n)), + Err(e) => parser_err!(format!("Could not parse '{}' as i64: {}", n, e)), + }, + Token::SingleQuotedString(ref s) => Ok(Value::SingleQuotedString(s.to_string())), + Token::NationalStringLiteral(ref s) => { + Ok(Value::NationalStringLiteral(s.to_string())) } - } + _ => parser_err!(format!("Unsupported value: {:?}", t)), + }, None => parser_err!("Expecting a value, but found EOF"), } } @@ -985,86 +976,6 @@ impl Parser { } } - pub fn parse_timezone_offset(&mut self) -> Result { - match self.next_token() { - Some(Token::Plus) => { - let n = self.parse_literal_int()?; - Ok(n as i8) - } - Some(Token::Minus) => { - let n = self.parse_literal_int()?; - Ok(-n as i8) - } - other => parser_err!(format!( - "Expecting `+` or `-` in timezone, but found {:?}", - other - )), - } - } - - pub fn parse_timestamp_value(&mut self) -> Result { - let year = self.parse_literal_int()?; - let date = self.parse_date(year)?; - if let Ok(time) = self.parse_time() { - let date_time = NaiveDateTime::new(date, time); - match self.peek_token() { - Some(token) => match token { - Token::Plus | Token::Minus => { - let tz = self.parse_timezone_offset()?; - let offset = FixedOffset::east(i32::from(tz) * 3600); - Ok(Value::Timestamp(DateTime::from_utc(date_time, offset))) - } - _ => Ok(Value::DateTime(date_time)), - }, - _ => Ok(Value::DateTime(date_time)), - } - } else { - parser_err!(format!( - "Expecting time after date, but found {:?}", - self.peek_token() - )) - } - } - - pub fn parse_date(&mut self, year: i64) -> Result { - if self.consume_token(&Token::Minus) { - let month = self.parse_literal_int()?; - if self.consume_token(&Token::Minus) { - let day = self.parse_literal_int()?; - let date = NaiveDate::from_ymd(year as i32, month as u32, day as u32); - Ok(date) - } else { - parser_err!(format!( - "Expecting `-` for date separator, found {:?}", - self.peek_token() - )) - } - } else { - parser_err!(format!( - "Expecting `-` for date separator, found {:?}", - self.peek_token() - )) - } - } - - pub fn parse_time(&mut self) -> Result { - let hour = self.parse_literal_int()?; - self.expect_token(&Token::Colon)?; - let min = self.parse_literal_int()?; - self.expect_token(&Token::Colon)?; - // On one hand, the SQL specs defines ::= , - // so it would be more correct to parse it as such - let sec = self.parse_literal_double()?; - // On the other, chrono only supports nanoseconds, which should(?) fit in seconds-as-f64... - let nanos = (sec.fract() * 1_000_000_000.0).round(); - Ok(NaiveTime::from_hms_nano( - hour as u32, - min as u32, - sec as u32, - nanos as u32, - )) - } - /// Parse a SQL datatype (in the context of a CREATE TABLE statement for example) pub fn parse_data_type(&mut self) -> Result { match self.next_token() { diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index e9354020..56d8bb8c 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -209,22 +209,6 @@ PHP ₱ USD $ //assert_eq!(sql, ast.to_string()); } -#[test] -fn parse_timestamps_example() { - let sql = "2016-02-15 09:43:33"; - let _ = parse_sql_expr(sql); - //TODO add assertion - //assert_eq!(sql, ast.to_string()); -} - -#[test] -fn parse_timestamps_with_millis_example() { - let sql = "2017-11-02 19:15:42.308637"; - let _ = parse_sql_expr(sql); - //TODO add assertion - //assert_eq!(sql, ast.to_string()); -} - fn verified_stmt(query: &str) -> SQLStatement { one_statement_parses_to(query, query) } @@ -247,12 +231,6 @@ fn parse_sql_statements(sql: &str) -> Result, ParserError> { Parser::parse_sql(&PostgreSqlDialect {}, sql.to_string()) } -fn parse_sql_expr(sql: &str) -> ASTNode { - debug!("sql: {}", sql); - let mut parser = parser(sql); - parser.parse_expr().unwrap() -} - fn parser(sql: &str) -> Parser { let dialect = PostgreSqlDialect {}; let mut tokenizer = Tokenizer::new(&dialect, &sql); From 478dbe940d071b84b9a373456c197dd6b4d1dfa8 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Mon, 29 Apr 2019 01:43:41 +0300 Subject: [PATCH 07/11] Factor test helpers into a common module Also run "generic" tests with all dialects (`parse_select_version` doesn't work with ANSI dialect, so I moved it to the postgres file temporarily) --- src/dialect/ansi_sql.rs | 1 + src/dialect/generic_sql.rs | 2 + src/dialect/mod.rs | 4 +- src/dialect/postgresql.rs | 1 + src/lib.rs | 5 + src/test_utils.rs | 121 ++++++++++++++++++ ...lparser_generic.rs => sqlparser_common.rs} | 101 ++++----------- tests/sqlparser_postgres.rs | 76 +++++------ 8 files changed, 197 insertions(+), 114 deletions(-) create mode 100644 src/test_utils.rs rename tests/{sqlparser_generic.rs => sqlparser_common.rs} (94%) diff --git a/src/dialect/ansi_sql.rs b/src/dialect/ansi_sql.rs index 522ed0ef..939f8546 100644 --- a/src/dialect/ansi_sql.rs +++ b/src/dialect/ansi_sql.rs @@ -1,5 +1,6 @@ use crate::dialect::Dialect; +#[derive(Debug)] pub struct AnsiSqlDialect {} impl Dialect for AnsiSqlDialect { diff --git a/src/dialect/generic_sql.rs b/src/dialect/generic_sql.rs index fe48ab2d..3674eae7 100644 --- a/src/dialect/generic_sql.rs +++ b/src/dialect/generic_sql.rs @@ -1,4 +1,6 @@ use crate::dialect::Dialect; + +#[derive(Debug)] pub struct GenericSqlDialect {} impl Dialect for GenericSqlDialect { diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index 95ecf792..dff51cda 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -3,11 +3,13 @@ mod generic_sql; pub mod keywords; mod postgresql; +use std::fmt::Debug; + pub use self::ansi_sql::AnsiSqlDialect; pub use self::generic_sql::GenericSqlDialect; pub use self::postgresql::PostgreSqlDialect; -pub trait Dialect { +pub trait Dialect: Debug { /// Determine if a character starts a quoted identifier. The default /// implementation, accepting "double quoted" ids is both ANSI-compliant /// and appropriate for most dialects (with the notable exception of diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs index dac3740d..757a7119 100644 --- a/src/dialect/postgresql.rs +++ b/src/dialect/postgresql.rs @@ -1,5 +1,6 @@ use crate::dialect::Dialect; +#[derive(Debug)] pub struct PostgreSqlDialect {} impl Dialect for PostgreSqlDialect { diff --git a/src/lib.rs b/src/lib.rs index fc90be85..420e9c3f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -40,3 +40,8 @@ pub mod dialect; pub mod sqlast; pub mod sqlparser; pub mod sqltokenizer; + +#[doc(hidden)] +// This is required to make utilities accessible by both the crate-internal +// unit-tests and by the integration tests +pub mod test_utils; diff --git a/src/test_utils.rs b/src/test_utils.rs new file mode 100644 index 00000000..fdefe097 --- /dev/null +++ b/src/test_utils.rs @@ -0,0 +1,121 @@ +use std::fmt::Debug; + +use super::dialect::*; +use super::sqlast::*; +use super::sqlparser::{Parser, ParserError}; +use super::sqltokenizer::Tokenizer; + +/// Tests use the methods on this struct to invoke the parser on one or +/// multiple dialects. +pub struct TestedDialects { + pub dialects: Vec>, +} + +impl TestedDialects { + /// Run the given function for all of `self.dialects`, assert that they + /// return the same result, and return that result. + pub fn one_of_identical_results(&self, f: F) -> T + where + F: Fn(&dyn Dialect) -> T, + { + let parse_results = self.dialects.iter().map(|dialect| (dialect, f(&**dialect))); + parse_results + .fold(None, |s, (dialect, parsed)| { + if let Some((prev_dialect, prev_parsed)) = s { + assert_eq!( + prev_parsed, parsed, + "Parse results with {:?} are different from {:?}", + prev_dialect, dialect + ); + } + Some((dialect, parsed)) + }) + .unwrap() + .1 + } + + pub fn run_parser_method(&self, sql: &str, f: F) -> T + where + F: Fn(&mut Parser) -> T, + { + self.one_of_identical_results(|dialect| { + let mut tokenizer = Tokenizer::new(dialect, sql); + let tokens = tokenizer.tokenize().unwrap(); + f(&mut Parser::new(tokens)) + }) + } + + pub fn parse_sql_statements(&self, sql: &str) -> Result, ParserError> { + self.one_of_identical_results(|dialect| Parser::parse_sql(dialect, sql.to_string())) + // To fail the `ensure_multiple_dialects_are_tested` test: + // Parser::parse_sql(&**self.dialects.first().unwrap(), sql.to_string()) + } + + /// Ensures that `sql` parses as a single statement, optionally checking + /// that converting AST back to string equals to `canonical` (unless an + /// empty canonical string is provided). + pub fn one_statement_parses_to(&self, sql: &str, canonical: &str) -> SQLStatement { + let mut statements = self.parse_sql_statements(&sql).unwrap(); + assert_eq!(statements.len(), 1); + + let only_statement = statements.pop().unwrap(); + if !canonical.is_empty() { + assert_eq!(canonical, only_statement.to_string()) + } + only_statement + } + + /// Ensures that `sql` parses as a single SQLStatement, and is not modified + /// after a serialization round-trip. + pub fn verified_stmt(&self, query: &str) -> SQLStatement { + self.one_statement_parses_to(query, query) + } + + /// Ensures that `sql` parses as a single SQLQuery, and is not modified + /// after a serialization round-trip. + pub fn verified_query(&self, sql: &str) -> SQLQuery { + match self.verified_stmt(sql) { + SQLStatement::SQLQuery(query) => *query, + _ => panic!("Expected SQLQuery"), + } + } + + /// Ensures that `sql` parses as a single SQLSelect, and is not modified + /// after a serialization round-trip. + pub fn verified_only_select(&self, query: &str) -> SQLSelect { + match self.verified_query(query).body { + SQLSetExpr::Select(s) => *s, + _ => panic!("Expected SQLSetExpr::Select"), + } + } + + /// Ensures that `sql` parses as an expression, and is not modified + /// after a serialization round-trip. + pub fn verified_expr(&self, sql: &str) -> ASTNode { + let ast = self.run_parser_method(sql, Parser::parse_expr).unwrap(); + assert_eq!(sql, &ast.to_string(), "round-tripping without changes"); + ast + } +} + +pub fn all_dialects() -> TestedDialects { + TestedDialects { + dialects: vec![ + Box::new(GenericSqlDialect {}), + Box::new(PostgreSqlDialect {}), + Box::new(AnsiSqlDialect {}), + ], + } +} + +pub fn only(v: &[T]) -> &T { + assert_eq!(1, v.len()); + v.first().unwrap() +} + +pub fn expr_from_projection(item: &SQLSelectItem) -> &ASTNode { + match item { + SQLSelectItem::UnnamedExpression(expr) => expr, + _ => panic!("Expected UnnamedExpression"), + } +} diff --git a/tests/sqlparser_generic.rs b/tests/sqlparser_common.rs similarity index 94% rename from tests/sqlparser_generic.rs rename to tests/sqlparser_common.rs index 633b5c1c..d8ce26bb 100644 --- a/tests/sqlparser_generic.rs +++ b/tests/sqlparser_common.rs @@ -2,10 +2,9 @@ use matches::assert_matches; -use sqlparser::dialect::*; use sqlparser::sqlast::*; -use sqlparser::sqlparser::*; -use sqlparser::sqltokenizer::*; +use sqlparser::sqlparser::ParserError; +use sqlparser::test_utils::{all_dialects, expr_from_projection, only}; #[test] fn parse_insert_values() { @@ -695,16 +694,6 @@ fn parse_simple_math_expr_minus() { verified_only_select(sql); } -#[test] -fn parse_select_version() { - let sql = "SELECT @@version"; - let select = verified_only_select(sql); - assert_eq!( - &ASTNode::SQLIdentifier("@@version".to_string()), - expr_from_projection(only(&select.projection)), - ); -} - #[test] fn parse_delimited_identifiers() { // check that quoted identifiers in any position remain quoted after serialization @@ -1169,73 +1158,35 @@ fn parse_invalid_subquery_without_parens() { ); } -fn only(v: &[T]) -> &T { - assert_eq!(1, v.len()); - v.first().unwrap() -} - -fn verified_query(query: &str) -> SQLQuery { - match verified_stmt(query) { - SQLStatement::SQLQuery(query) => *query, - _ => panic!("Expected SQLQuery"), - } -} - -fn expr_from_projection(item: &SQLSelectItem) -> &ASTNode { - match item { - SQLSelectItem::UnnamedExpression(expr) => expr, - _ => panic!("Expected UnnamedExpression"), - } -} - -fn verified_only_select(query: &str) -> SQLSelect { - match verified_query(query).body { - SQLSetExpr::Select(s) => *s, - _ => panic!("Expected SQLSetExpr::Select"), - } -} - -fn verified_stmt(query: &str) -> SQLStatement { - one_statement_parses_to(query, query) -} - -fn verified_expr(query: &str) -> ASTNode { - let ast = parse_sql_expr(query); - assert_eq!(query, &ast.to_string()); - ast -} - -/// Ensures that `sql` parses as a single statement, optionally checking that -/// converting AST back to string equals to `canonical` (unless an empty string -/// is provided). -fn one_statement_parses_to(sql: &str, canonical: &str) -> SQLStatement { - let mut statements = parse_sql_statements(&sql).unwrap(); - assert_eq!(statements.len(), 1); - - let only_statement = statements.pop().unwrap(); - if !canonical.is_empty() { - assert_eq!(canonical, only_statement.to_string()) - } - only_statement +#[test] +#[should_panic(expected = "Parse results with PostgreSqlDialect are different from AnsiSqlDialect")] +fn ensure_multiple_dialects_are_tested() { + // The SQL here must be parsed differently by different dialects. + // At the time of writing, `@foo` is accepted as a valid identifier + // by the generic and the postgresql dialect, but not by the ANSI one. + let _ = parse_sql_statements("SELECT @foo"); } fn parse_sql_statements(sql: &str) -> Result, ParserError> { - let generic_ast = Parser::parse_sql(&GenericSqlDialect {}, sql.to_string()); - let pg_ast = Parser::parse_sql(&PostgreSqlDialect {}, sql.to_string()); - assert_eq!(generic_ast, pg_ast); - generic_ast + all_dialects().parse_sql_statements(sql) } -fn parse_sql_expr(sql: &str) -> ASTNode { - let generic_ast = parse_sql_expr_with(&GenericSqlDialect {}, &sql.to_string()); - let pg_ast = parse_sql_expr_with(&PostgreSqlDialect {}, &sql.to_string()); - assert_eq!(generic_ast, pg_ast); - generic_ast +fn one_statement_parses_to(sql: &str, canonical: &str) -> SQLStatement { + all_dialects().one_statement_parses_to(sql, canonical) } -fn parse_sql_expr_with(dialect: &dyn Dialect, sql: &str) -> ASTNode { - let mut tokenizer = Tokenizer::new(dialect, &sql); - let tokens = tokenizer.tokenize().unwrap(); - let mut parser = Parser::new(tokens); - parser.parse_expr().unwrap() +fn verified_stmt(query: &str) -> SQLStatement { + all_dialects().verified_stmt(query) +} + +fn verified_query(query: &str) -> SQLQuery { + all_dialects().verified_query(query) +} + +fn verified_only_select(query: &str) -> SQLSelect { + all_dialects().verified_only_select(query) +} + +fn verified_expr(query: &str) -> ASTNode { + all_dialects().verified_expr(query) } diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 56d8bb8c..fe5c92aa 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -1,39 +1,47 @@ #![warn(clippy::all)] -use log::debug; - -use sqlparser::dialect::PostgreSqlDialect; +use sqlparser::dialect::{GenericSqlDialect, PostgreSqlDialect}; use sqlparser::sqlast::*; use sqlparser::sqlparser::*; use sqlparser::sqltokenizer::*; +use sqlparser::test_utils::*; #[test] fn test_prev_index() { let sql = "SELECT version()"; - let mut parser = parser(sql); - assert_eq!(parser.prev_token(), None); - assert_eq!(parser.next_token(), Some(Token::make_keyword("SELECT"))); - assert_eq!(parser.next_token(), Some(Token::make_word("version", None))); - assert_eq!(parser.prev_token(), Some(Token::make_word("version", None))); - assert_eq!(parser.peek_token(), Some(Token::make_word("version", None))); - assert_eq!(parser.prev_token(), Some(Token::make_keyword("SELECT"))); - assert_eq!(parser.prev_token(), None); + all_dialects().run_parser_method(sql, |parser| { + assert_eq!(parser.prev_token(), None); + assert_eq!(parser.next_token(), Some(Token::make_keyword("SELECT"))); + assert_eq!(parser.next_token(), Some(Token::make_word("version", None))); + assert_eq!(parser.prev_token(), Some(Token::make_word("version", None))); + assert_eq!(parser.peek_token(), Some(Token::make_word("version", None))); + assert_eq!(parser.prev_token(), Some(Token::make_keyword("SELECT"))); + assert_eq!(parser.prev_token(), None); + }); } #[test] fn parse_invalid_table_name() { - let mut parser = parser("db.public..customer"); - let ast = parser.parse_object_name(); + let ast = all_dialects().run_parser_method("db.public..customer", Parser::parse_object_name); assert!(ast.is_err()); } #[test] fn parse_no_table_name() { - let mut parser = parser(""); - let ast = parser.parse_object_name(); + let ast = all_dialects().run_parser_method("", Parser::parse_object_name); assert!(ast.is_err()); } +#[test] +fn parse_select_version() { + let sql = "SELECT @@version"; + let select = pg_and_generic().verified_only_select(sql); + assert_eq!( + &ASTNode::SQLIdentifier("@@version".to_string()), + expr_from_projection(only(&select.projection)), + ); +} + #[test] fn parse_create_table_with_defaults() { let sql = "CREATE TABLE public.customer ( @@ -209,32 +217,24 @@ PHP ₱ USD $ //assert_eq!(sql, ast.to_string()); } -fn verified_stmt(query: &str) -> SQLStatement { - one_statement_parses_to(query, query) -} - -/// Ensures that `sql` parses as a single statement, optionally checking that -/// converting AST back to string equals to `canonical` (unless an empty string -/// is provided). fn one_statement_parses_to(sql: &str, canonical: &str) -> SQLStatement { - let mut statements = parse_sql_statements(&sql).unwrap(); - assert_eq!(statements.len(), 1); + pg().one_statement_parses_to(sql, canonical) +} +fn verified_stmt(query: &str) -> SQLStatement { + pg().verified_stmt(query) +} - let only_statement = statements.pop().unwrap(); - if !canonical.is_empty() { - assert_eq!(canonical, only_statement.to_string()) +fn pg() -> TestedDialects { + TestedDialects { + dialects: vec![Box::new(PostgreSqlDialect {})], } - only_statement } -fn parse_sql_statements(sql: &str) -> Result, ParserError> { - Parser::parse_sql(&PostgreSqlDialect {}, sql.to_string()) -} - -fn parser(sql: &str) -> Parser { - let dialect = PostgreSqlDialect {}; - let mut tokenizer = Tokenizer::new(&dialect, &sql); - let tokens = tokenizer.tokenize().unwrap(); - debug!("tokens: {:#?}", tokens); - Parser::new(tokens) +fn pg_and_generic() -> TestedDialects { + TestedDialects { + dialects: vec![ + Box::new(PostgreSqlDialect {}), + Box::new(GenericSqlDialect {}), + ], + } } From 1347ca08253895301dc3304e386da09c8581b75b Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Fri, 3 May 2019 02:42:18 +0300 Subject: [PATCH 08/11] Move the rest of tests not specific to PG from the sqlparser_postgres.rs --- src/sqlparser.rs | 20 ++++++++++++++++++++ tests/sqlparser_common.rs | 14 +++++++++++++- tests/sqlparser_postgres.rs | 27 --------------------------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/sqlparser.rs b/src/sqlparser.rs index b268040a..ab2755fc 100644 --- a/src/sqlparser.rs +++ b/src/sqlparser.rs @@ -1582,3 +1582,23 @@ impl SQLWord { self.to_string() } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_utils::all_dialects; + + #[test] + fn test_prev_index() { + let sql = "SELECT version()"; + all_dialects().run_parser_method(sql, |parser| { + assert_eq!(parser.prev_token(), None); + assert_eq!(parser.next_token(), Some(Token::make_keyword("SELECT"))); + assert_eq!(parser.next_token(), Some(Token::make_word("version", None))); + assert_eq!(parser.prev_token(), Some(Token::make_word("version", None))); + assert_eq!(parser.peek_token(), Some(Token::make_word("version", None))); + assert_eq!(parser.prev_token(), Some(Token::make_keyword("SELECT"))); + assert_eq!(parser.prev_token(), None); + }); + } +} diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index d8ce26bb..72e06404 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -3,7 +3,7 @@ use matches::assert_matches; use sqlparser::sqlast::*; -use sqlparser::sqlparser::ParserError; +use sqlparser::sqlparser::*; use sqlparser::test_utils::{all_dialects, expr_from_projection, only}; #[test] @@ -58,6 +58,18 @@ fn parse_insert_invalid() { ); } +#[test] +fn parse_invalid_table_name() { + let ast = all_dialects().run_parser_method("db.public..customer", Parser::parse_object_name); + assert!(ast.is_err()); +} + +#[test] +fn parse_no_table_name() { + let ast = all_dialects().run_parser_method("", Parser::parse_object_name); + assert!(ast.is_err()); +} + #[test] fn parse_delete_statement() { let sql = "DELETE FROM \"table\""; diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index fe5c92aa..f83bb308 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -3,35 +3,8 @@ use sqlparser::dialect::{GenericSqlDialect, PostgreSqlDialect}; use sqlparser::sqlast::*; use sqlparser::sqlparser::*; -use sqlparser::sqltokenizer::*; use sqlparser::test_utils::*; -#[test] -fn test_prev_index() { - let sql = "SELECT version()"; - all_dialects().run_parser_method(sql, |parser| { - assert_eq!(parser.prev_token(), None); - assert_eq!(parser.next_token(), Some(Token::make_keyword("SELECT"))); - assert_eq!(parser.next_token(), Some(Token::make_word("version", None))); - assert_eq!(parser.prev_token(), Some(Token::make_word("version", None))); - assert_eq!(parser.peek_token(), Some(Token::make_word("version", None))); - assert_eq!(parser.prev_token(), Some(Token::make_keyword("SELECT"))); - assert_eq!(parser.prev_token(), None); - }); -} - -#[test] -fn parse_invalid_table_name() { - let ast = all_dialects().run_parser_method("db.public..customer", Parser::parse_object_name); - assert!(ast.is_err()); -} - -#[test] -fn parse_no_table_name() { - let ast = all_dialects().run_parser_method("", Parser::parse_object_name); - assert!(ast.is_err()); -} - #[test] fn parse_select_version() { let sql = "SELECT @@version"; From 5047f2c02ea25469aed1d58f646452dd7e13a2e7 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Fri, 3 May 2019 02:52:01 +0300 Subject: [PATCH 09/11] Remove the ansi-specific test file and update PG tests - The ANSI dialect is now tested in `sqlparser_common.rs` - Some PG testcases are also parsed by the generic dialect successfully, so test that. --- tests/sqlparser_ansi.rs | 24 ------------------------ tests/sqlparser_postgres.rs | 16 ++++------------ 2 files changed, 4 insertions(+), 36 deletions(-) delete mode 100644 tests/sqlparser_ansi.rs diff --git a/tests/sqlparser_ansi.rs b/tests/sqlparser_ansi.rs deleted file mode 100644 index ab80ae92..00000000 --- a/tests/sqlparser_ansi.rs +++ /dev/null @@ -1,24 +0,0 @@ -#![warn(clippy::all)] - -use sqlparser::dialect::AnsiSqlDialect; -use sqlparser::sqlast::*; -use sqlparser::sqlparser::*; - -#[test] -fn parse_simple_select() { - let sql = String::from("SELECT id, fname, lname FROM customer WHERE id = 1"); - let mut ast = Parser::parse_sql(&AnsiSqlDialect {}, sql).unwrap(); - assert_eq!(1, ast.len()); - match ast.pop().unwrap() { - SQLStatement::SQLQuery(q) => match *q { - SQLQuery { - body: SQLSetExpr::Select(select), - .. - } => { - assert_eq!(3, select.projection.len()); - } - _ => unreachable!(), - }, - _ => unreachable!(), - } -} diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index f83bb308..557b7a98 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -2,7 +2,6 @@ use sqlparser::dialect::{GenericSqlDialect, PostgreSqlDialect}; use sqlparser::sqlast::*; -use sqlparser::sqlparser::*; use sqlparser::test_utils::*; #[test] @@ -28,7 +27,7 @@ fn parse_create_table_with_defaults() { create_date date DEFAULT now()::text NOT NULL, last_update timestamp without time zone DEFAULT now() NOT NULL, active integer NOT NULL)"; - match one_statement_parses_to(sql, "") { + match pg_and_generic().one_statement_parses_to(sql, "") { SQLStatement::SQLCreateTable { name, columns, @@ -74,7 +73,7 @@ fn parse_create_table_from_pg_dump() { release_year public.year, active integer )"; - match one_statement_parses_to(sql, "") { + match pg().one_statement_parses_to(sql, "") { SQLStatement::SQLCreateTable { name, columns, @@ -135,7 +134,7 @@ fn parse_create_table_with_inherit() { value text[], \ use_metric boolean DEFAULT true\ )"; - match verified_stmt(sql) { + match pg().verified_stmt(sql) { SQLStatement::SQLCreateTable { name, columns, @@ -185,18 +184,11 @@ Kwara & Kogi PHP ₱ USD $ \N Some other value \\."#; - let ast = one_statement_parses_to(sql, ""); + let ast = pg_and_generic().one_statement_parses_to(sql, ""); println!("{:#?}", ast); //assert_eq!(sql, ast.to_string()); } -fn one_statement_parses_to(sql: &str, canonical: &str) -> SQLStatement { - pg().one_statement_parses_to(sql, canonical) -} -fn verified_stmt(query: &str) -> SQLStatement { - pg().verified_stmt(query) -} - fn pg() -> TestedDialects { TestedDialects { dialects: vec![Box::new(PostgreSqlDialect {})], From 304710d59ad7b89378148eafec02c2045daaf826 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Fri, 3 May 2019 03:09:44 +0300 Subject: [PATCH 10/11] Add MSSQL dialect and fix up the postgres' identifier rules The `@@version` test is MS' dialect of SQL, it seems, so test it with its own dialect. Update the rules for identifiers in Postresql dialect per documentation, while we're at it. The current identifier rules in Postgresql dialect were introduced in this commit - as a copy of generic rules, it seems: https://github.com/andygrove/sqlparser-rs/commit/810cd8e6cf28a1586579bbd5d7dfcbae078c2922#diff-2808df0fba0aed85f9d35c167bd6a5f1L138 --- src/dialect/generic_sql.rs | 4 +++- src/dialect/mod.rs | 2 ++ src/dialect/mssql.rs | 22 +++++++++++++++++++++ src/dialect/postgresql.rs | 7 +++++-- src/test_utils.rs | 1 + tests/sqlparser_common.rs | 6 ++++-- tests/sqlparser_mssql.rs | 38 +++++++++++++++++++++++++++++++++++++ tests/sqlparser_postgres.rs | 10 ---------- 8 files changed, 75 insertions(+), 15 deletions(-) create mode 100644 src/dialect/mssql.rs create mode 100644 tests/sqlparser_mssql.rs diff --git a/src/dialect/generic_sql.rs b/src/dialect/generic_sql.rs index 3674eae7..09de4745 100644 --- a/src/dialect/generic_sql.rs +++ b/src/dialect/generic_sql.rs @@ -5,7 +5,7 @@ pub struct GenericSqlDialect {} impl Dialect for GenericSqlDialect { fn is_identifier_start(&self, ch: char) -> bool { - (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || ch == '@' + (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || ch == '_' || ch == '#' || ch == '@' } fn is_identifier_part(&self, ch: char) -> bool { @@ -13,6 +13,8 @@ impl Dialect for GenericSqlDialect { || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '@' + || ch == '$' + || ch == '#' || ch == '_' } } diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index dff51cda..df6f2603 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -1,12 +1,14 @@ mod ansi_sql; mod generic_sql; pub mod keywords; +mod mssql; mod postgresql; use std::fmt::Debug; pub use self::ansi_sql::AnsiSqlDialect; pub use self::generic_sql::GenericSqlDialect; +pub use self::mssql::MsSqlDialect; pub use self::postgresql::PostgreSqlDialect; pub trait Dialect: Debug { diff --git a/src/dialect/mssql.rs b/src/dialect/mssql.rs new file mode 100644 index 00000000..65daeb8d --- /dev/null +++ b/src/dialect/mssql.rs @@ -0,0 +1,22 @@ +use crate::dialect::Dialect; + +#[derive(Debug)] +pub struct MsSqlDialect {} + +impl Dialect for MsSqlDialect { + fn is_identifier_start(&self, ch: char) -> bool { + // See https://docs.microsoft.com/en-us/sql/relational-databases/databases/database-identifiers?view=sql-server-2017#rules-for-regular-identifiers + // We don't support non-latin "letters" currently. + (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || ch == '_' || ch == '#' || ch == '@' + } + + fn is_identifier_part(&self, ch: char) -> bool { + (ch >= 'a' && ch <= 'z') + || (ch >= 'A' && ch <= 'Z') + || (ch >= '0' && ch <= '9') + || ch == '@' + || ch == '$' + || ch == '#' + || ch == '_' + } +} diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs index 757a7119..5433b440 100644 --- a/src/dialect/postgresql.rs +++ b/src/dialect/postgresql.rs @@ -5,14 +5,17 @@ pub struct PostgreSqlDialect {} impl Dialect for PostgreSqlDialect { fn is_identifier_start(&self, ch: char) -> bool { - (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || ch == '@' + // See https://www.postgresql.org/docs/11/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS + // We don't yet support identifiers beginning with "letters with + // diacritical marks and non-Latin letters" + (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || ch == '_' } fn is_identifier_part(&self, ch: char) -> bool { (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') - || ch == '@' + || ch == '$' || ch == '_' } } diff --git a/src/test_utils.rs b/src/test_utils.rs index fdefe097..16216bfe 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -103,6 +103,7 @@ pub fn all_dialects() -> TestedDialects { dialects: vec![ Box::new(GenericSqlDialect {}), Box::new(PostgreSqlDialect {}), + Box::new(MsSqlDialect {}), Box::new(AnsiSqlDialect {}), ], } diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 72e06404..177d5824 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -1171,11 +1171,13 @@ fn parse_invalid_subquery_without_parens() { } #[test] -#[should_panic(expected = "Parse results with PostgreSqlDialect are different from AnsiSqlDialect")] +#[should_panic( + expected = "Parse results with GenericSqlDialect are different from PostgreSqlDialect" +)] fn ensure_multiple_dialects_are_tested() { // The SQL here must be parsed differently by different dialects. // At the time of writing, `@foo` is accepted as a valid identifier - // by the generic and the postgresql dialect, but not by the ANSI one. + // by the Generic and the MSSQL dialect, but not by Postgres and ANSI. let _ = parse_sql_statements("SELECT @foo"); } diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs new file mode 100644 index 00000000..d0fcab9e --- /dev/null +++ b/tests/sqlparser_mssql.rs @@ -0,0 +1,38 @@ +#![warn(clippy::all)] + +use sqlparser::dialect::{GenericSqlDialect, MsSqlDialect}; +use sqlparser::sqlast::*; +use sqlparser::test_utils::*; + +#[test] +fn parse_mssql_identifiers() { + let sql = "SELECT @@version, _foo$123 FROM ##temp"; + let select = ms_and_generic().verified_only_select(sql); + assert_eq!( + &ASTNode::SQLIdentifier("@@version".to_string()), + expr_from_projection(&select.projection[0]), + ); + assert_eq!( + &ASTNode::SQLIdentifier("_foo$123".to_string()), + expr_from_projection(&select.projection[1]), + ); + assert_eq!(2, select.projection.len()); + match select.relation { + Some(TableFactor::Table { name, .. }) => { + assert_eq!("##temp".to_string(), name.to_string()); + } + _ => unreachable!(), + }; +} + +#[allow(dead_code)] +fn ms() -> TestedDialects { + TestedDialects { + dialects: vec![Box::new(MsSqlDialect {})], + } +} +fn ms_and_generic() -> TestedDialects { + TestedDialects { + dialects: vec![Box::new(MsSqlDialect {}), Box::new(GenericSqlDialect {})], + } +} diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 557b7a98..3a6f1d40 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -4,16 +4,6 @@ use sqlparser::dialect::{GenericSqlDialect, PostgreSqlDialect}; use sqlparser::sqlast::*; use sqlparser::test_utils::*; -#[test] -fn parse_select_version() { - let sql = "SELECT @@version"; - let select = pg_and_generic().verified_only_select(sql); - assert_eq!( - &ASTNode::SQLIdentifier("@@version".to_string()), - expr_from_projection(only(&select.projection)), - ); -} - #[test] fn parse_create_table_with_defaults() { let sql = "CREATE TABLE public.customer ( From 67cc880fd1631f8211bf95f6d2e4a0c4fa00f886 Mon Sep 17 00:00:00 2001 From: Nickolay Ponomarev Date: Sat, 4 May 2019 02:42:22 +0300 Subject: [PATCH 11/11] Add comments to the test files --- tests/sqlparser_common.rs | 6 ++++++ tests/sqlparser_mssql.rs | 2 ++ tests/sqlparser_postgres.rs | 2 ++ 3 files changed, 10 insertions(+) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 177d5824..14498ce2 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -1,4 +1,10 @@ #![warn(clippy::all)] +//! Test SQL syntax, which all sqlparser dialects must parse in the same way. +//! +//! Note that it does not mean all SQL here is valid in all the dialects, only +//! that 1) it's either standard or widely supported and 2) it can be parsed by +//! sqlparser regardless of the chosen dialect (i.e. it doesn't conflict with +//! dialect-specific parsing rules). use matches::assert_matches; diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs index d0fcab9e..d209ed94 100644 --- a/tests/sqlparser_mssql.rs +++ b/tests/sqlparser_mssql.rs @@ -1,4 +1,6 @@ #![warn(clippy::all)] +//! Test SQL syntax specific to Microsoft's T-SQL. The parser based on the +//! generic dialect is also tested (on the inputs it can handle). use sqlparser::dialect::{GenericSqlDialect, MsSqlDialect}; use sqlparser::sqlast::*; diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 3a6f1d40..522bd74f 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -1,4 +1,6 @@ #![warn(clippy::all)] +//! Test SQL syntax specific to PostgreSQL. The parser based on the +//! generic dialect is also tested (on the inputs it can handle). use sqlparser::dialect::{GenericSqlDialect, PostgreSqlDialect}; use sqlparser::sqlast::*;