From 653cd2bf6f35f9a9aa84b0cb93fcf071063e499e Mon Sep 17 00:00:00 2001 From: Sibz <12050358+Sibz@users.noreply.github.com> Date: Sat, 16 Jul 2022 19:22:34 +1000 Subject: [PATCH 1/2] fix: mysql ON UPDATE not taking an expression closes #543 --- src/ast/ddl.rs | 3 +++ src/parser.rs | 25 ++++++++++++++++++++++--- tests/sqlparser_mysql.rs | 19 +++++++++++++++---- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 00bf83aba3..976cc6d1bb 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -540,6 +540,8 @@ pub enum ColumnOption { /// - MySQL's `AUTO_INCREMENT` or SQLite's `AUTOINCREMENT` /// - ... DialectSpecific(Vec), + /// MySql specific ON UPDATE + OnUpdate(Expr), CharacterSet(ObjectName), Comment(String), } @@ -576,6 +578,7 @@ impl fmt::Display for ColumnOption { DialectSpecific(val) => write!(f, "{}", display_separated(val, " ")), CharacterSet(n) => write!(f, "CHARACTER SET {}", n), Comment(v) => write!(f, "COMMENT '{}'", escape_single_quote_string(v)), + OnUpdate(expr) => write!(f, "ON UPDATE {}", expr), } } } diff --git a/src/parser.rs b/src/parser.rs index 528b8c6fe6..c676467377 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3254,9 +3254,9 @@ impl<'a> Parser<'a> { } else if self.parse_keywords(&[Keyword::ON, Keyword::UPDATE]) && dialect_of!(self is MySqlDialect) { - Ok(Some(ColumnOption::DialectSpecific(vec![ - Token::make_keyword("ON UPDATE"), - ]))) + let expr = self.parse_expr()?; + Self::check_on_update_expr_is_valid(&expr)?; + Ok(Some(ColumnOption::OnUpdate(expr))) } else { Ok(None) } @@ -6318,6 +6318,25 @@ impl<'a> Parser<'a> { pub fn index(&self) -> usize { self.index } + + fn check_on_update_expr_is_valid(expr: &Expr) -> Result<(), ParserError> { + let valid_object_names: [ObjectName; 4] = [ + ObjectName(vec!["CURRENT_TIMESTAMP".into()]), + ObjectName(vec!["LOCALTIME".into()]), + ObjectName(vec!["LOCALTIMESTAMP".into()]), + ObjectName(vec!["NOW".into()]), + ]; + + if let Expr::Function(f) = expr { + if valid_object_names.contains(&f.name) { + return Ok(()); + } + } + Err(ParserError::ParserError(format!( + "Expected one of '{}' after ON UPDATE in column definition", + valid_object_names.map(|x| x.to_string()).join("', '") + ))) + } } impl Word { diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 67850b1b93..eb71ed3269 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -1031,7 +1031,7 @@ fn parse_kill() { #[test] fn parse_table_colum_option_on_update() { - let sql1 = "CREATE TABLE foo (`modification_time` DATETIME ON UPDATE)"; + let sql1 = "CREATE TABLE foo (`modification_time` DATETIME ON UPDATE CURRENT_TIMESTAMP())"; match mysql().verified_stmt(sql1) { Statement::CreateTable { name, columns, .. } => { assert_eq!(name.to_string(), "foo"); @@ -1042,9 +1042,13 @@ fn parse_table_colum_option_on_update() { collation: None, options: vec![ColumnOptionDef { name: None, - option: ColumnOption::DialectSpecific(vec![Token::make_keyword( - "ON UPDATE" - )]), + option: ColumnOption::OnUpdate(Expr::Function(Function { + name: ObjectName(vec!["CURRENT_TIMESTAMP".into()]), + args: vec![], + over: None, + distinct: false, + special: false, + })), },], }], columns @@ -1054,6 +1058,13 @@ fn parse_table_colum_option_on_update() { } } +#[test] +fn parse_table_colum_option_on_update_error() { + let sql1 = "CREATE TABLE foo (`modification_time` DATETIME ON UPDATE BOO())"; + assert_eq!(mysql().parse_sql_statements(sql1).unwrap_err(), + sqlparser::parser::ParserError::ParserError("Expected one of 'CURRENT_TIMESTAMP', 'LOCALTIME', 'LOCALTIMESTAMP', 'NOW' after ON UPDATE in column definition".to_string())); +} + #[test] fn parse_set_names() { let stmt = mysql_and_generic().verified_stmt("SET NAMES utf8mb4"); From e6b0c646deec825424c4660b3eb439dee9e96d45 Mon Sep 17 00:00:00 2001 From: Sibz <12050358+Sibz@users.noreply.github.com> Date: Wed, 21 Dec 2022 12:07:08 +1100 Subject: [PATCH 2/2] refactor: removed allocation in check fn parse::check_on_update_expr_is_valid - using local const instead of vec for valid object names --- src/keywords.rs | 1 + src/parser.rs | 27 ++++++++++++++++----------- tests/sqlparser_mysql.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/keywords.rs b/src/keywords.rs index d41463e9e1..6e1df3e726 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -383,6 +383,7 @@ define_keywords!( NOSUPERUSER, NOT, NOTHING, + NOW, NOWAIT, NTH_VALUE, NTILE, diff --git a/src/parser.rs b/src/parser.rs index c676467377..e79032e7d6 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -497,9 +497,8 @@ impl<'a> Parser<'a> { | Keyword::CURRENT_TIME | Keyword::CURRENT_DATE | Keyword::LOCALTIME - | Keyword::LOCALTIMESTAMP => { - self.parse_time_functions(ObjectName(vec![w.to_ident()])) - } + | Keyword::LOCALTIMESTAMP + | Keyword::NOW => self.parse_time_functions(ObjectName(vec![w.to_ident()])), Keyword::CASE => self.parse_case_expr(), Keyword::CAST => self.parse_cast_expr(), Keyword::TRY_CAST => self.parse_try_cast_expr(), @@ -6320,21 +6319,27 @@ impl<'a> Parser<'a> { } fn check_on_update_expr_is_valid(expr: &Expr) -> Result<(), ParserError> { - let valid_object_names: [ObjectName; 4] = [ - ObjectName(vec!["CURRENT_TIMESTAMP".into()]), - ObjectName(vec!["LOCALTIME".into()]), - ObjectName(vec!["LOCALTIMESTAMP".into()]), - ObjectName(vec!["NOW".into()]), + const VALID_FN_NAMES: [&str; 4] = [ + keywords::CURRENT_TIMESTAMP, + keywords::LOCALTIME, + keywords::LOCALTIMESTAMP, + keywords::NOW, ]; if let Expr::Function(f) = expr { - if valid_object_names.contains(&f.name) { - return Ok(()); + if let Some(fn_name_ident) = f.name.0.first() { + if VALID_FN_NAMES + .iter() + .any(|x| x.eq_ignore_ascii_case(&fn_name_ident.value.as_str())) + { + return Ok(()); + } } } + Err(ParserError::ParserError(format!( "Expected one of '{}' after ON UPDATE in column definition", - valid_object_names.map(|x| x.to_string()).join("', '") + VALID_FN_NAMES.map(|x| x.to_string()).join("', '") ))) } } diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index eb71ed3269..c909af65fd 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -1058,6 +1058,35 @@ fn parse_table_colum_option_on_update() { } } +#[test] +fn parse_table_colum_option_on_update_now_lowercase() { + let sql1 = "CREATE TABLE foo (`modification_time` DATETIME ON UPDATE now())"; + match mysql().verified_stmt(sql1) { + Statement::CreateTable { name, columns, .. } => { + assert_eq!(name.to_string(), "foo"); + assert_eq!( + vec![ColumnDef { + name: Ident::with_quote('`', "modification_time"), + data_type: DataType::Datetime(None), + collation: None, + options: vec![ColumnOptionDef { + name: None, + option: ColumnOption::OnUpdate(Expr::Function(Function { + name: ObjectName(vec!["now".into()]), + args: vec![], + over: None, + distinct: false, + special: false, + })), + },], + }], + columns + ); + } + _ => unreachable!(), + } +} + #[test] fn parse_table_colum_option_on_update_error() { let sql1 = "CREATE TABLE foo (`modification_time` DATETIME ON UPDATE BOO())";