From d7727a1f46c1eea36f17aa9f8268be22f3597b67 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sun, 4 Feb 2024 23:24:46 -0800 Subject: [PATCH 01/18] Restrain JSON_TABLE to MySqlDialect --- derive/src/lib.rs | 62 ++++++++++++++++++++++++++++++----------------- src/parser/mod.rs | 4 ++- 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/derive/src/lib.rs b/derive/src/lib.rs index d19696aa4..92b50315c 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -3,29 +3,34 @@ use quote::{format_ident, quote, quote_spanned, ToTokens}; use syn::spanned::Spanned; use syn::{ parse::{Parse, ParseStream}, - parse_macro_input, parse_quote, Attribute, Data, DeriveInput, - Fields, GenericParam, Generics, Ident, Index, LitStr, Meta, Token + parse_macro_input, parse_quote, Attribute, Data, DeriveInput, Fields, GenericParam, Generics, + Ident, Index, LitStr, Meta, Token, }; - /// Implementation of `[#derive(Visit)]` #[proc_macro_derive(VisitMut, attributes(visit))] pub fn derive_visit_mut(input: proc_macro::TokenStream) -> proc_macro::TokenStream { - derive_visit(input, &VisitType { - visit_trait: quote!(VisitMut), - visitor_trait: quote!(VisitorMut), - modifier: Some(quote!(mut)), - }) + derive_visit( + input, + &VisitType { + visit_trait: quote!(VisitMut), + visitor_trait: quote!(VisitorMut), + modifier: Some(quote!(mut)), + }, + ) } /// Implementation of `[#derive(Visit)]` #[proc_macro_derive(Visit, attributes(visit))] pub fn derive_visit_immutable(input: proc_macro::TokenStream) -> proc_macro::TokenStream { - derive_visit(input, &VisitType { - visit_trait: quote!(Visit), - visitor_trait: quote!(Visitor), - modifier: None, - }) + derive_visit( + input, + &VisitType { + visit_trait: quote!(Visit), + visitor_trait: quote!(Visitor), + modifier: None, + }, + ) } struct VisitType { @@ -34,15 +39,16 @@ struct VisitType { modifier: Option, } -fn derive_visit( - input: proc_macro::TokenStream, - visit_type: &VisitType, -) -> proc_macro::TokenStream { +fn derive_visit(input: proc_macro::TokenStream, visit_type: &VisitType) -> proc_macro::TokenStream { // Parse the input tokens into a syntax tree. let input = parse_macro_input!(input as DeriveInput); let name = input.ident; - let VisitType { visit_trait, visitor_trait, modifier } = visit_type; + let VisitType { + visit_trait, + visitor_trait, + modifier, + } = visit_type; let attributes = Attributes::parse(&input.attrs); // Add a bound `T: Visit` to every type parameter T. @@ -87,7 +93,10 @@ impl Parse for WithIdent { let mut result = WithIdent { with: None }; let ident = input.parse::()?; if ident != "with" { - return Err(syn::Error::new(ident.span(), "Expected identifier to be `with`")); + return Err(syn::Error::new( + ident.span(), + "Expected identifier to be `with`", + )); } input.parse::()?; let s = input.parse::()?; @@ -131,17 +140,26 @@ impl Attributes { } // Add a bound `T: Visit` to every type parameter T. -fn add_trait_bounds(mut generics: Generics, VisitType{visit_trait, ..}: &VisitType) -> Generics { +fn add_trait_bounds(mut generics: Generics, VisitType { visit_trait, .. }: &VisitType) -> Generics { for param in &mut generics.params { if let GenericParam::Type(ref mut type_param) = *param { - type_param.bounds.push(parse_quote!(sqlparser::ast::#visit_trait)); + type_param + .bounds + .push(parse_quote!(sqlparser::ast::#visit_trait)); } } generics } // Generate the body of the visit implementation for the given type -fn visit_children(data: &Data, VisitType{visit_trait, modifier, ..}: &VisitType) -> TokenStream { +fn visit_children( + data: &Data, + VisitType { + visit_trait, + modifier, + .. + }: &VisitType, +) -> TokenStream { match data { Data::Struct(data) => match &data.fields { Fields::Named(fields) => { diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 9d79a32e3..4a98ea542 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7506,7 +7506,9 @@ impl<'a> Parser<'a> { with_offset, with_offset_alias, }) - } else if self.parse_keyword(Keyword::JSON_TABLE) { + } else if dialect_of!(self is MySqlDialect) + && self.parse_keyword(Keyword::JSON_TABLE) + { self.expect_token(&Token::LParen)?; let json_expr = self.parse_expr()?; self.expect_token(&Token::Comma)?; From 5f73e1a7500852ae6b09fa5c58eb7cc09288c66d Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Mon, 5 Feb 2024 07:17:47 -0800 Subject: [PATCH 02/18] Add test for generic dialect --- src/parser/mod.rs | 4 +--- tests/sqlparser_common.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 4a98ea542..c307fe91b 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7506,9 +7506,7 @@ impl<'a> Parser<'a> { with_offset, with_offset_alias, }) - } else if dialect_of!(self is MySqlDialect) - && self.parse_keyword(Keyword::JSON_TABLE) - { + } else if dialect_of!(self is MySqlDialect) && self.parse_keyword(Keyword::JSON_TABLE) { self.expect_token(&Token::LParen)?; let json_expr = self.parse_expr()?; self.expect_token(&Token::Comma)?; diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 691ffb92d..8ec80e358 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8407,3 +8407,16 @@ fn test_buffer_reuse() { p.parse_statements().unwrap(); let _ = p.into_tokens(); } + +#[test] +fn parse_json_table() { + let dialects = TestedDialects { + dialects: vec![Box::new(GenericDialect {})], + options: None, + }; + + // JSON_TABLE is not supported in the generic dialect. + assert!(dialects + .parse_sql_statements("SELECT * FROM JSON_TABLE('[[1, 2], [3, 4]]', '$[*]' COLUMNS(a INT PATH '$[0]', b INT PATH '$[1]')) AS t") + .is_err()); +} From be776e52fbb5123275446604ca9cdbbd38aa054d Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 8 Feb 2024 06:34:25 -0800 Subject: [PATCH 03/18] Restore rustfmt change --- derive/src/lib.rs | 62 +++++++++++++++++------------------------------ 1 file changed, 22 insertions(+), 40 deletions(-) diff --git a/derive/src/lib.rs b/derive/src/lib.rs index 92b50315c..d19696aa4 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -3,34 +3,29 @@ use quote::{format_ident, quote, quote_spanned, ToTokens}; use syn::spanned::Spanned; use syn::{ parse::{Parse, ParseStream}, - parse_macro_input, parse_quote, Attribute, Data, DeriveInput, Fields, GenericParam, Generics, - Ident, Index, LitStr, Meta, Token, + parse_macro_input, parse_quote, Attribute, Data, DeriveInput, + Fields, GenericParam, Generics, Ident, Index, LitStr, Meta, Token }; + /// Implementation of `[#derive(Visit)]` #[proc_macro_derive(VisitMut, attributes(visit))] pub fn derive_visit_mut(input: proc_macro::TokenStream) -> proc_macro::TokenStream { - derive_visit( - input, - &VisitType { - visit_trait: quote!(VisitMut), - visitor_trait: quote!(VisitorMut), - modifier: Some(quote!(mut)), - }, - ) + derive_visit(input, &VisitType { + visit_trait: quote!(VisitMut), + visitor_trait: quote!(VisitorMut), + modifier: Some(quote!(mut)), + }) } /// Implementation of `[#derive(Visit)]` #[proc_macro_derive(Visit, attributes(visit))] pub fn derive_visit_immutable(input: proc_macro::TokenStream) -> proc_macro::TokenStream { - derive_visit( - input, - &VisitType { - visit_trait: quote!(Visit), - visitor_trait: quote!(Visitor), - modifier: None, - }, - ) + derive_visit(input, &VisitType { + visit_trait: quote!(Visit), + visitor_trait: quote!(Visitor), + modifier: None, + }) } struct VisitType { @@ -39,16 +34,15 @@ struct VisitType { modifier: Option, } -fn derive_visit(input: proc_macro::TokenStream, visit_type: &VisitType) -> proc_macro::TokenStream { +fn derive_visit( + input: proc_macro::TokenStream, + visit_type: &VisitType, +) -> proc_macro::TokenStream { // Parse the input tokens into a syntax tree. let input = parse_macro_input!(input as DeriveInput); let name = input.ident; - let VisitType { - visit_trait, - visitor_trait, - modifier, - } = visit_type; + let VisitType { visit_trait, visitor_trait, modifier } = visit_type; let attributes = Attributes::parse(&input.attrs); // Add a bound `T: Visit` to every type parameter T. @@ -93,10 +87,7 @@ impl Parse for WithIdent { let mut result = WithIdent { with: None }; let ident = input.parse::()?; if ident != "with" { - return Err(syn::Error::new( - ident.span(), - "Expected identifier to be `with`", - )); + return Err(syn::Error::new(ident.span(), "Expected identifier to be `with`")); } input.parse::()?; let s = input.parse::()?; @@ -140,26 +131,17 @@ impl Attributes { } // Add a bound `T: Visit` to every type parameter T. -fn add_trait_bounds(mut generics: Generics, VisitType { visit_trait, .. }: &VisitType) -> Generics { +fn add_trait_bounds(mut generics: Generics, VisitType{visit_trait, ..}: &VisitType) -> Generics { for param in &mut generics.params { if let GenericParam::Type(ref mut type_param) = *param { - type_param - .bounds - .push(parse_quote!(sqlparser::ast::#visit_trait)); + type_param.bounds.push(parse_quote!(sqlparser::ast::#visit_trait)); } } generics } // Generate the body of the visit implementation for the given type -fn visit_children( - data: &Data, - VisitType { - visit_trait, - modifier, - .. - }: &VisitType, -) -> TokenStream { +fn visit_children(data: &Data, VisitType{visit_trait, modifier, ..}: &VisitType) -> TokenStream { match data { Data::Struct(data) => match &data.fields { Fields::Named(fields) => { From bed4af39fe4f0d44d006f285382a42218c264581 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 8 Feb 2024 08:18:55 -0800 Subject: [PATCH 04/18] Add test --- src/parser/mod.rs | 2 +- tests/sqlparser_common.rs | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index c307fe91b..7f918e2c1 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7506,7 +7506,7 @@ impl<'a> Parser<'a> { with_offset, with_offset_alias, }) - } else if dialect_of!(self is MySqlDialect) && self.parse_keyword(Keyword::JSON_TABLE) { + } else if dialect_of!(self is MySqlDialect | AnsiDialect) && self.parse_keyword(Keyword::JSON_TABLE) { self.expect_token(&Token::LParen)?; let json_expr = self.parse_expr()?; self.expect_token(&Token::Comma)?; diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 8ec80e358..5d05bd5f8 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8409,7 +8409,7 @@ fn test_buffer_reuse() { } #[test] -fn parse_json_table() { +fn parse_json_table_err() { let dialects = TestedDialects { dialects: vec![Box::new(GenericDialect {})], options: None, @@ -8420,3 +8420,12 @@ fn parse_json_table() { .parse_sql_statements("SELECT * FROM JSON_TABLE('[[1, 2], [3, 4]]', '$[*]' COLUMNS(a INT PATH '$[0]', b INT PATH '$[1]')) AS t") .is_err()); } + +#[test] +fn parse_json_table_as_identifier() { + let supported_dialects = all_dialects_except(|d| d.is::() || d.is::()); + + assert!(supported_dialects + .parse_sql_statements("SELECT * FROM json_table") + .is_ok()); +} From d7fdae5f3e1a6480e7ea3bafc11ae4a91a645193 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 8 Feb 2024 08:22:26 -0800 Subject: [PATCH 05/18] Add more dialects to negative test --- tests/sqlparser_common.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 5d05bd5f8..511b58fdf 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8410,13 +8410,10 @@ fn test_buffer_reuse() { #[test] fn parse_json_table_err() { - let dialects = TestedDialects { - dialects: vec![Box::new(GenericDialect {})], - options: None, - }; + let unsupported_dialects = all_dialects_except(|d| d.is::() || d.is::()); - // JSON_TABLE is not supported in the generic dialect. - assert!(dialects + // JSON_TABLE table function is not supported in the above dialects. + assert!(unsupported_dialects .parse_sql_statements("SELECT * FROM JSON_TABLE('[[1, 2], [3, 4]]', '$[*]' COLUMNS(a INT PATH '$[0]', b INT PATH '$[1]')) AS t") .is_err()); } From 6febd52b11e4b2eb1c46e2f6591ac097ff103b60 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 8 Feb 2024 08:59:55 -0800 Subject: [PATCH 06/18] Fix format --- src/parser/mod.rs | 4 +++- tests/sqlparser_common.rs | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 7f918e2c1..1f3424765 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7506,7 +7506,9 @@ impl<'a> Parser<'a> { with_offset, with_offset_alias, }) - } else if dialect_of!(self is MySqlDialect | AnsiDialect) && self.parse_keyword(Keyword::JSON_TABLE) { + } else if dialect_of!(self is MySqlDialect | AnsiDialect) + && self.parse_keyword(Keyword::JSON_TABLE) + { self.expect_token(&Token::LParen)?; let json_expr = self.parse_expr()?; self.expect_token(&Token::Comma)?; diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 511b58fdf..aad5e1e7d 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8410,7 +8410,8 @@ fn test_buffer_reuse() { #[test] fn parse_json_table_err() { - let unsupported_dialects = all_dialects_except(|d| d.is::() || d.is::()); + let unsupported_dialects = + all_dialects_except(|d| d.is::() || d.is::()); // JSON_TABLE table function is not supported in the above dialects. assert!(unsupported_dialects @@ -8420,7 +8421,8 @@ fn parse_json_table_err() { #[test] fn parse_json_table_as_identifier() { - let supported_dialects = all_dialects_except(|d| d.is::() || d.is::()); + let supported_dialects = + all_dialects_except(|d| d.is::() || d.is::()); assert!(supported_dialects .parse_sql_statements("SELECT * FROM json_table") From 0b225bf380e90001c8779842ed251da9b4ec535f Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Fri, 9 Feb 2024 15:19:03 -0800 Subject: [PATCH 07/18] More --- src/ast/mod.rs | 17 +++++++++++++++++ src/parser/mod.rs | 36 ++++++++++++++++++++++++++++++++++-- tests/sqlparser_common.rs | 15 ++++++++------- 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 371fe3d54..1a13ba11e 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -23,6 +23,7 @@ use core::fmt::{self, Display}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; +use sqlparser::keywords::{ALL_KEYWORDS, ALL_KEYWORDS_INDEX}; #[cfg(feature = "visitor")] use sqlparser_derive::{Visit, VisitMut}; @@ -54,6 +55,7 @@ pub use self::value::{ use crate::ast::helpers::stmt_data_loading::{ DataLoadingOptions, StageLoadSelectItem, StageParamsObject, }; +use crate::keywords::Keyword; #[cfg(feature = "visitor")] pub use visitor::*; @@ -141,6 +143,21 @@ impl Ident { quote_style: Some(quote), } } + + /// Returns the defined `Keyword` enum for this identifier if it is a keyword. + pub fn find_keyword(&self) -> Option { + ALL_KEYWORDS + .iter() + .enumerate() + .find_map(|(idx, &kw)| { + if kw.to_string().to_uppercase() == self.value.to_uppercase() { + Some(idx) + } else { + None + } + }) + .map(|idx| ALL_KEYWORDS_INDEX[idx]) + } } impl From<&str> for Ident { diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 1f3424765..0b3a3892d 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2771,6 +2771,27 @@ impl<'a> Parser<'a> { } } + /// If the current token is the `expected` keyword followed by + /// specified tokens, and consume them and returns true. + /// Otherwise, no tokens are consumed and returns false. + pub fn parse_keyword_with_tokens(&mut self, expected: Keyword, tokens: &[Token]) -> bool { + let index = self.index; + match self.peek_token().token { + Token::Word(w) if expected == w.keyword => { + self.next_token(); + for token in tokens { + if !self.consume_token(token) { + // reset index and return immediately + self.index = index; + return false; + } + } + true + } + _ => false, + } + } + /// If the current and subsequent tokens exactly match the `keywords` /// sequence, consume them and returns true. Otherwise, no tokens are /// consumed and returns false @@ -7507,9 +7528,8 @@ impl<'a> Parser<'a> { with_offset_alias, }) } else if dialect_of!(self is MySqlDialect | AnsiDialect) - && self.parse_keyword(Keyword::JSON_TABLE) + && self.parse_keyword_with_tokens(Keyword::JSON_TABLE, &[Token::LParen]) { - self.expect_token(&Token::LParen)?; let json_expr = self.parse_expr()?; self.expect_token(&Token::Comma)?; let json_path = self.parse_value()?; @@ -7526,8 +7546,20 @@ impl<'a> Parser<'a> { alias, }) } else { + let loc = self.peek_token().location; let name = self.parse_object_name(true)?; + for ident in &name.0 { + if ident.quote_style.is_none() { + if ident.find_keyword().is_some() { + return parser_err!( + "Cannot specify a reserved keyword as identifier of table factor", + loc + ); + } + } + } + let partitions: Vec = if dialect_of!(self is MySqlDialect | GenericDialect) && self.parse_keyword(Keyword::PARTITION) { diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index aad5e1e7d..0ba2a00b8 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8409,7 +8409,7 @@ fn test_buffer_reuse() { } #[test] -fn parse_json_table_err() { +fn parse_json_table_function_err() { let unsupported_dialects = all_dialects_except(|d| d.is::() || d.is::()); @@ -8421,10 +8421,11 @@ fn parse_json_table_err() { #[test] fn parse_json_table_as_identifier() { - let supported_dialects = - all_dialects_except(|d| d.is::() || d.is::()); - - assert!(supported_dialects - .parse_sql_statements("SELECT * FROM json_table") - .is_ok()); + let parsed = all_dialects().parse_sql_statements("SELECT * FROM json_table"); + assert_eq!( + ParserError::ParserError( + "Cannot specify a reserved keyword as identifier of table factor".to_string() + ), + parsed.unwrap_err() + ); } From e1e5c419b5eef6be3d2e919ed8f94cd5428806e4 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Fri, 9 Feb 2024 15:21:22 -0800 Subject: [PATCH 08/18] Fix --- src/parser/mod.rs | 2 +- tests/sqlparser_common.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 0b3a3892d..5240a4207 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7553,7 +7553,7 @@ impl<'a> Parser<'a> { if ident.quote_style.is_none() { if ident.find_keyword().is_some() { return parser_err!( - "Cannot specify a reserved keyword as identifier of table factor", + "Cannot specify a reserved keyword as identifier for table factor", loc ); } diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 0ba2a00b8..69bffb8d7 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8424,7 +8424,7 @@ fn parse_json_table_as_identifier() { let parsed = all_dialects().parse_sql_statements("SELECT * FROM json_table"); assert_eq!( ParserError::ParserError( - "Cannot specify a reserved keyword as identifier of table factor".to_string() + "Cannot specify a reserved keyword as identifier for table factor".to_string() ), parsed.unwrap_err() ); From b7a4d3bcf8cb9bceac574d6a13991d426f9577d6 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Fri, 9 Feb 2024 15:23:05 -0800 Subject: [PATCH 09/18] Fix --- src/parser/mod.rs | 2 +- tests/sqlparser_common.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 5240a4207..a9279a8ef 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7553,7 +7553,7 @@ impl<'a> Parser<'a> { if ident.quote_style.is_none() { if ident.find_keyword().is_some() { return parser_err!( - "Cannot specify a reserved keyword as identifier for table factor", + "Cannot specify a keyword as identifier for table factor", loc ); } diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 69bffb8d7..b26e2dfc1 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8424,7 +8424,7 @@ fn parse_json_table_as_identifier() { let parsed = all_dialects().parse_sql_statements("SELECT * FROM json_table"); assert_eq!( ParserError::ParserError( - "Cannot specify a reserved keyword as identifier for table factor".to_string() + "Cannot specify a keyword as identifier for table factor".to_string() ), parsed.unwrap_err() ); From 80a3dda1c8e7ec00ba88752b18b00a9df1aed00d Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Fri, 9 Feb 2024 15:27:54 -0800 Subject: [PATCH 10/18] Cleanup --- src/parser/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index a9279a8ef..59dcb6017 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2800,7 +2800,6 @@ impl<'a> Parser<'a> { let index = self.index; for &keyword in keywords { if !self.parse_keyword(keyword) { - // println!("parse_keywords aborting .. did not find {:?}", keyword); // reset index and return immediately self.index = index; return false; From 7f7d764beb2890132d1cfd1b323ecf7d08561671 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sun, 11 Feb 2024 09:58:49 -0800 Subject: [PATCH 11/18] Update src/parser/mod.rs Co-authored-by: Andrew Lamb --- src/parser/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 59dcb6017..3ec421e35 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2772,7 +2772,7 @@ impl<'a> Parser<'a> { } /// If the current token is the `expected` keyword followed by - /// specified tokens, and consume them and returns true. + /// specified tokens, consume them and returns true. /// Otherwise, no tokens are consumed and returns false. pub fn parse_keyword_with_tokens(&mut self, expected: Keyword, tokens: &[Token]) -> bool { let index = self.index; From b27da0fb5289c71e4dd104ef1de918d88ff1c55a Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sun, 11 Feb 2024 09:59:37 -0800 Subject: [PATCH 12/18] Update src/ast/mod.rs Co-authored-by: Andrew Lamb --- src/ast/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 1a13ba11e..d0cddd70e 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -144,7 +144,10 @@ impl Ident { } } - /// Returns the defined `Keyword` enum for this identifier if it is a keyword. + /// If this identifier is also a keyword, return the corresponding [`Keyword`]. + /// + /// For example even though `AVRO` is a keyword, it can also be used as an + /// identifier for a column, such as `SELECT avro FROM my_table`. pub fn find_keyword(&self) -> Option { ALL_KEYWORDS .iter() From 545c66101616b19b4695f4020275a6e583a7c09e Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sun, 11 Feb 2024 10:23:34 -0800 Subject: [PATCH 13/18] Remove redundant whitespace --- src/ast/mod.rs | 2 +- src/parser/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index d0cddd70e..212b1378e 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -145,7 +145,7 @@ impl Ident { } /// If this identifier is also a keyword, return the corresponding [`Keyword`]. - /// + /// /// For example even though `AVRO` is a keyword, it can also be used as an /// identifier for a column, such as `SELECT avro FROM my_table`. pub fn find_keyword(&self) -> Option { diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 3ec421e35..5fef54897 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2772,7 +2772,7 @@ impl<'a> Parser<'a> { } /// If the current token is the `expected` keyword followed by - /// specified tokens, consume them and returns true. + /// specified tokens, consume them and returns true. /// Otherwise, no tokens are consumed and returns false. pub fn parse_keyword_with_tokens(&mut self, expected: Keyword, tokens: &[Token]) -> bool { let index = self.index; From 96ecfb1c84dadc0ea951da886790d4adf56126ae Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sun, 11 Feb 2024 10:48:10 -0800 Subject: [PATCH 14/18] For review and fix tests --- src/parser/mod.rs | 12 ++++++------ tests/sqlparser_bigquery.rs | 10 +++++----- tests/sqlparser_clickhouse.rs | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 5fef54897..3443cff39 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2775,17 +2775,17 @@ impl<'a> Parser<'a> { /// specified tokens, consume them and returns true. /// Otherwise, no tokens are consumed and returns false. pub fn parse_keyword_with_tokens(&mut self, expected: Keyword, tokens: &[Token]) -> bool { - let index = self.index; match self.peek_token().token { Token::Word(w) if expected == w.keyword => { - self.next_token(); - for token in tokens { - if !self.consume_token(token) { - // reset index and return immediately - self.index = index; + for (idx, token) in tokens.iter().enumerate() { + if self.peek_nth_token(idx + 1).token != *token { return false; } } + // consume all tokens + for _ in 0..(tokens.len() + 1) { + self.next_token(); + } true } _ => false, diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index 5e79db33a..f2bb3d723 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -866,13 +866,13 @@ fn parse_table_identifiers() { vec![Ident::with_quote('`', "GROUP"), Ident::new("dataField")], ); - // TODO: this should be error - // test_table_ident_err("GROUP.dataField"); + test_table_ident_err("GROUP.dataField"); + test_table_ident_err("abc5.GROUP"); test_table_ident( - "abc5.GROUP", + "abc5.`GROUP`", None, - vec![Ident::new("abc5"), Ident::new("GROUP")], + vec![Ident::new("abc5"), Ident::with_quote('`', "GROUP")], ); test_table_ident( @@ -1205,7 +1205,7 @@ fn parse_array_agg_func() { #[test] fn test_select_wildcard_with_except() { - let select = bigquery_and_generic().verified_only_select("SELECT * EXCEPT (col_a) FROM data"); + let select = bigquery_and_generic().verified_only_select("SELECT * EXCEPT (col_a) FROM `data`"); let expected = SelectItem::Wildcard(WildcardAdditionalOptions { opt_except: Some(ExceptSelectItem { first_element: Ident::new("col_a"), diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index e7c85c2a3..daea30bed 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -356,10 +356,10 @@ fn parse_double_equal() { #[test] fn parse_limit_by() { clickhouse_and_generic().verified_stmt( - r#"SELECT * FROM default.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset"#, + r#"SELECT * FROM `default`.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset"#, ); clickhouse_and_generic().verified_stmt( - r#"SELECT * FROM default.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset, toStartOfDay(created_at)"#, + r#"SELECT * FROM `default`.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset, toStartOfDay(created_at)"#, ); } From 8ce05b3ac77fadec739497ed0c70c8749fe766cc Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Mon, 12 Feb 2024 21:33:22 -0800 Subject: [PATCH 15/18] Fix clippy --- src/parser/mod.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 3443cff39..c219b467a 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7549,13 +7549,11 @@ impl<'a> Parser<'a> { let name = self.parse_object_name(true)?; for ident in &name.0 { - if ident.quote_style.is_none() { - if ident.find_keyword().is_some() { - return parser_err!( - "Cannot specify a keyword as identifier for table factor", - loc - ); - } + if ident.quote_style.is_none() && ident.find_keyword().is_some() { + return parser_err!( + "Cannot specify a keyword as identifier for table factor", + loc + ); } } From cd10ba55e4c05300239188214320c7adf0cb52d1 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Tue, 13 Feb 2024 09:57:12 -0800 Subject: [PATCH 16/18] Only prevent keyword as identifier under ANSI mode --- src/parser/mod.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index c219b467a..25f3f46b6 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7548,12 +7548,18 @@ impl<'a> Parser<'a> { let loc = self.peek_token().location; let name = self.parse_object_name(true)?; - for ident in &name.0 { - if ident.quote_style.is_none() && ident.find_keyword().is_some() { - return parser_err!( - "Cannot specify a keyword as identifier for table factor", - loc - ); + // Prevents using keywords as identifiers for table factor in ANSI mode + if dialect_of!(self is AnsiDialect) { + for ident in &name.0 { + if ident.quote_style.is_none() && ident.find_keyword().is_some() { + return parser_err!( + format!( + "Cannot specify a keyword as identifier for table factor: {}", + ident.value + ), + loc + ); + } } } From 7b803a3d07f14bc6a286dd4773dce0d8252ead67 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Tue, 13 Feb 2024 09:59:33 -0800 Subject: [PATCH 17/18] Restore test change --- tests/sqlparser_bigquery.rs | 10 +++++----- tests/sqlparser_clickhouse.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index f2bb3d723..5e79db33a 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -866,13 +866,13 @@ fn parse_table_identifiers() { vec![Ident::with_quote('`', "GROUP"), Ident::new("dataField")], ); - test_table_ident_err("GROUP.dataField"); - test_table_ident_err("abc5.GROUP"); + // TODO: this should be error + // test_table_ident_err("GROUP.dataField"); test_table_ident( - "abc5.`GROUP`", + "abc5.GROUP", None, - vec![Ident::new("abc5"), Ident::with_quote('`', "GROUP")], + vec![Ident::new("abc5"), Ident::new("GROUP")], ); test_table_ident( @@ -1205,7 +1205,7 @@ fn parse_array_agg_func() { #[test] fn test_select_wildcard_with_except() { - let select = bigquery_and_generic().verified_only_select("SELECT * EXCEPT (col_a) FROM `data`"); + let select = bigquery_and_generic().verified_only_select("SELECT * EXCEPT (col_a) FROM data"); let expected = SelectItem::Wildcard(WildcardAdditionalOptions { opt_except: Some(ExceptSelectItem { first_element: Ident::new("col_a"), diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs index daea30bed..e7c85c2a3 100644 --- a/tests/sqlparser_clickhouse.rs +++ b/tests/sqlparser_clickhouse.rs @@ -356,10 +356,10 @@ fn parse_double_equal() { #[test] fn parse_limit_by() { clickhouse_and_generic().verified_stmt( - r#"SELECT * FROM `default`.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset"#, + r#"SELECT * FROM default.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset"#, ); clickhouse_and_generic().verified_stmt( - r#"SELECT * FROM `default`.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset, toStartOfDay(created_at)"#, + r#"SELECT * FROM default.last_asset_runs_mv ORDER BY created_at DESC LIMIT 1 BY asset, toStartOfDay(created_at)"#, ); } From ff4c995ad6965c16aadf11d7bdd373fa180a3a51 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Tue, 13 Feb 2024 12:35:12 -0800 Subject: [PATCH 18/18] Fix test --- tests/sqlparser_common.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index b26e2dfc1..f3fcffdb9 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8421,10 +8421,17 @@ fn parse_json_table_function_err() { #[test] fn parse_json_table_as_identifier() { - let parsed = all_dialects().parse_sql_statements("SELECT * FROM json_table"); + let ansi_dialect = TestedDialects { + dialects: vec![ + Box::new(AnsiDialect {}), + ], + options: None, + }; + + let parsed = ansi_dialect.parse_sql_statements("SELECT * FROM json_table"); assert_eq!( ParserError::ParserError( - "Cannot specify a keyword as identifier for table factor".to_string() + "Cannot specify a keyword as identifier for table factor: json_table".to_string() ), parsed.unwrap_err() );