Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrain JSON_TABLE table function parsing to MySqlDialect and AnsiDialect #1123

Closed
wants to merge 19 commits into from
Closed
17 changes: 17 additions & 0 deletions src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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::*;

Expand Down Expand Up @@ -141,6 +143,21 @@ impl Ident {
quote_style: Some(quote),
}
}

/// Returns the defined `Keyword` enum for this identifier if it is a keyword.
viirya marked this conversation as resolved.
Show resolved Hide resolved
pub fn find_keyword(&self) -> Option<Keyword> {
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 {
Expand Down
38 changes: 36 additions & 2 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
viirya marked this conversation as resolved.
Show resolved Hide resolved
/// 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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I think it would be possible to call peek_nth_token here as well to avoid having to reset index. However, this works fine too

https://docs.rs/sqlparser/latest/sqlparser/parser/struct.Parser.html#method.peek_nth_token

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. Changed to use peek_nth_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
Expand Down Expand Up @@ -7506,8 +7527,9 @@ impl<'a> Parser<'a> {
with_offset,
with_offset_alias,
})
} else if self.parse_keyword(Keyword::JSON_TABLE) {
self.expect_token(&Token::LParen)?;
} else if dialect_of!(self is MySqlDialect | AnsiDialect)
&& self.parse_keyword_with_tokens(Keyword::JSON_TABLE, &[Token::LParen])
{
let json_expr = self.parse_expr()?;
self.expect_token(&Token::Comma)?;
let json_path = self.parse_value()?;
Expand All @@ -7524,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 keyword as identifier for table factor",
loc
);
}
}
}

let partitions: Vec<Ident> = if dialect_of!(self is MySqlDialect | GenericDialect)
&& self.parse_keyword(Keyword::PARTITION)
{
Expand Down
22 changes: 22 additions & 0 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8407,3 +8407,25 @@ fn test_buffer_reuse() {
p.parse_statements().unwrap();
let _ = p.into_tokens();
}

#[test]
fn parse_json_table_function_err() {
let unsupported_dialects =
all_dialects_except(|d| d.is::<AnsiDialect>() || d.is::<MySqlDialect>());

// 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());
}

#[test]
fn parse_json_table_as_identifier() {
let parsed = all_dialects().parse_sql_statements("SELECT * FROM json_table");
assert_eq!(
ParserError::ParserError(
"Cannot specify a keyword as identifier for table factor".to_string()
),
parsed.unwrap_err()
);
Copy link
Member Author

@viirya viirya Feb 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, even for the supported dialects, json_table cannot be used as table identifier too. Previously it cannot be too but the paring error looks confusing.

}