iffyio commented on code in PR #1501:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1835618993
##########
src/parser/mod.rs:
##########
@@ -12315,6 +12310,124 @@ impl<'a> Parser<'a> {
}
false
}
+
+ /// Look for all of the expected keywords in sequence, without consuming
them
+ fn peek_keywords(&mut self, expected: &[Keyword]) -> bool {
+ let index = self.index;
+ for kw in expected {
+ if !self.parse_keyword(*kw) {
+ self.index = index;
+ return false;
+ }
+ }
+ self.index = index;
+ true
+ }
+
+ fn parse_show_stmt_options(&mut self) -> Result<ShowStatementOptions,
ParserError> {
+ let show_in;
+ let mut filter_position = None;
+ if self.dialect.supports_show_like_before_in() {
+ if let Some(filter) = self.parse_show_statement_filter()? {
+ filter_position =
Some(ShowStatementFilterPosition::Infix(filter));
+ }
+ show_in = self.maybe_parse_show_stmt_in()?;
+ } else {
+ show_in = self.maybe_parse_show_stmt_in()?;
+ if let Some(filter) = self.parse_show_statement_filter()? {
+ filter_position =
Some(ShowStatementFilterPosition::Suffix(filter));
+ }
+ }
+ let starts_with = self.maybe_parse_show_stmt_starts_with()?;
+ let limit = self.maybe_parse_show_stmt_limit()?;
+ let from = self.maybe_parse_show_stmt_from()?;
+ Ok(ShowStatementOptions {
+ filter_position,
+ show_in,
+ starts_with,
+ limit,
+ limit_from: from,
+ })
+ }
+
+ fn maybe_parse_show_stmt_in(&mut self) -> Result<Option<ShowStatementIn>,
ParserError> {
+ let clause = match self.parse_one_of_keywords(&[Keyword::FROM,
Keyword::IN]) {
+ Some(Keyword::FROM) => ShowStatementInClause::FROM,
+ Some(Keyword::IN) => ShowStatementInClause::IN,
+ _ => return Ok(None),
+ };
+
+ let (parent_type, parent_name) = match self.parse_one_of_keywords(&[
+ Keyword::ACCOUNT,
+ Keyword::DATABASE,
+ Keyword::SCHEMA,
+ Keyword::TABLE,
+ Keyword::VIEW,
+ ]) {
+ Some(Keyword::DATABASE) if self.peek_keywords(&[Keyword::STARTS,
Keyword::WITH]) => {
+ (Some(ShowStatementInParentType::Database), None)
+ }
+ Some(Keyword::SCHEMA) if self.peek_keywords(&[Keyword::STARTS,
Keyword::WITH]) => {
+ (Some(ShowStatementInParentType::Schema), None)
+ }
+ Some(parent_kw) => {
+ let parent_name = match self.parse_object_name(false) {
+ Ok(n) => Some(n),
+ _ => None,
+ };
+ match parent_kw {
+ Keyword::ACCOUNT =>
(Some(ShowStatementInParentType::Account), parent_name),
+ Keyword::DATABASE =>
(Some(ShowStatementInParentType::Database), parent_name),
+ Keyword::SCHEMA =>
(Some(ShowStatementInParentType::Schema), parent_name),
+ Keyword::TABLE => (Some(ShowStatementInParentType::Table),
parent_name),
+ Keyword::VIEW => (Some(ShowStatementInParentType::View),
parent_name),
+ _ => unreachable!(),
Review Comment:
I think similarly here, for unexpected states, ideally we can return an
error to fail the parsing that specific input in order to avoid crashing
##########
src/ast/mod.rs:
##########
@@ -7357,6 +7354,99 @@ impl Display for UtilityOption {
}
}
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+pub struct ShowStatementOptions {
+ pub show_in: Option<ShowStatementIn>,
+ pub starts_with: Option<Value>,
+ pub limit: Option<Expr>,
+ pub limit_from: Option<Value>,
+ pub filter_position: Option<ShowStatementFilterPosition>,
+}
+
+impl Display for ShowStatementOptions {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ let (life_in_infix, like_in_suffix) = match &self.filter_position {
Review Comment:
```suggestion
let (like_in_infix, like_in_suffix) = match &self.filter_position {
```
##########
tests/sqlparser_snowflake.rs:
##########
@@ -2781,3 +2781,68 @@ fn test_parentheses_overflow() {
snowflake_with_recursion_limit(max_nesting_level).parse_sql_statements(sql.as_str());
assert_eq!(parsed.err(), Some(ParserError::RecursionLimitExceeded));
}
+
+#[test]
+fn test_show_databases() {
+ snowflake().verified_stmt("SHOW DATABASES");
+ snowflake().verified_stmt("SHOW DATABASES HISTORY");
+ snowflake().verified_stmt("SHOW DATABASES LIKE '%abc%'");
+ snowflake().verified_stmt("SHOW DATABASES STARTS WITH 'demo_db'");
+ snowflake().verified_stmt("SHOW DATABASES LIMIT 12");
+ snowflake()
+ .verified_stmt("SHOW DATABASES HISTORY LIKE '%aa' STARTS WITH 'demo'
LIMIT 20 FROM 'abc'");
+ snowflake().verified_stmt("SHOW DATABASES IN ACCOUNT abc");
+}
+
+#[test]
+fn test_parse_show_schemas() {
+ snowflake().verified_stmt("SHOW SCHEMAS");
+ snowflake().verified_stmt("SHOW SCHEMAS IN ACCOUNT");
+ snowflake().verified_stmt("SHOW SCHEMAS IN ACCOUNT abc");
+ snowflake().verified_stmt("SHOW SCHEMAS IN DATABASE");
+ snowflake().verified_stmt("SHOW SCHEMAS IN DATABASE xyz");
+ snowflake().verified_stmt("SHOW SCHEMAS HISTORY LIKE '%xa%'");
+ snowflake().verified_stmt("SHOW SCHEMAS STARTS WITH 'abc' LIMIT 20");
+ snowflake().verified_stmt("SHOW SCHEMAS IN DATABASE STARTS WITH 'abc'
LIMIT 20 FROM 'xyz'");
+}
+
+#[test]
+fn test_parse_show_tables() {
+ snowflake().verified_stmt("SHOW TABLES");
Review Comment:
for these tests would it make sense to verify the statements using `let d =
all_dialects_where(|d| d.supports_show_like_before_in());`? so that any future
dialect with the same behavior as snowflake in this regard gets automatic test
coverage
##########
src/parser/mod.rs:
##########
@@ -12289,6 +12352,137 @@ impl<'a> Parser<'a> {
}
false
}
+
+ /// Look for an expected keyword, without consuming it
+ fn peek_keyword(&self, expected: Keyword) -> bool {
+ match self.peek_token().token {
+ Token::Word(w) => expected == w.keyword,
+ _ => false,
+ }
+ }
+
+ /// Look for one of expected keyword, without consuming it
+ fn peek_keywords(&self, expected: &[Keyword]) -> bool {
+ for kw in expected {
+ if self.peek_keyword(*kw) {
+ return true;
+ }
+ }
+ false
+ }
+
+ fn parse_show_opt_in(&mut self) -> Result<Option<ShowStatementIn>,
ParserError> {
+ let clause = match self.parse_one_of_keywords(&[Keyword::FROM,
Keyword::IN]) {
+ Some(Keyword::FROM) => ShowStatementInClause::FROM,
+ Some(Keyword::IN) => ShowStatementInClause::IN,
+ _ => return Ok(None),
Review Comment:
Ah right, yeah I think in this commit both the input and the behavior of
`parse_one_of_keywords` are in sync, with code changes there's a chance that
could change. One could introduce a bug like updating the input keywords and
forgetting to handle the branch. It would be desirable to return an error if
such ever occurs. We could make the noop branch here explicit in that case e.g?
```rust
let clause = match self.parse_one_of_keywords(&[Keyword::FROM, Keyword::IN])
{
Some(Keyword::FROM) => ShowStatementInClause::FROM,
Some(Keyword::IN) => ShowStatementInClause::IN,
None => return Ok(None),
_ => return self.expected("")
}
```
##########
src/parser/mod.rs:
##########
@@ -12315,6 +12310,124 @@ impl<'a> Parser<'a> {
}
false
}
+
+ /// Look for all of the expected keywords in sequence, without consuming
them
+ fn peek_keywords(&mut self, expected: &[Keyword]) -> bool {
+ let index = self.index;
+ for kw in expected {
+ if !self.parse_keyword(*kw) {
+ self.index = index;
+ return false;
+ }
+ }
+ self.index = index;
+ true
+ }
+
+ fn parse_show_stmt_options(&mut self) -> Result<ShowStatementOptions,
ParserError> {
+ let show_in;
+ let mut filter_position = None;
+ if self.dialect.supports_show_like_before_in() {
+ if let Some(filter) = self.parse_show_statement_filter()? {
+ filter_position =
Some(ShowStatementFilterPosition::Infix(filter));
+ }
+ show_in = self.maybe_parse_show_stmt_in()?;
+ } else {
+ show_in = self.maybe_parse_show_stmt_in()?;
+ if let Some(filter) = self.parse_show_statement_filter()? {
+ filter_position =
Some(ShowStatementFilterPosition::Suffix(filter));
+ }
+ }
+ let starts_with = self.maybe_parse_show_stmt_starts_with()?;
+ let limit = self.maybe_parse_show_stmt_limit()?;
+ let from = self.maybe_parse_show_stmt_from()?;
+ Ok(ShowStatementOptions {
+ filter_position,
+ show_in,
+ starts_with,
+ limit,
+ limit_from: from,
+ })
+ }
+
+ fn maybe_parse_show_stmt_in(&mut self) -> Result<Option<ShowStatementIn>,
ParserError> {
+ let clause = match self.parse_one_of_keywords(&[Keyword::FROM,
Keyword::IN]) {
+ Some(Keyword::FROM) => ShowStatementInClause::FROM,
+ Some(Keyword::IN) => ShowStatementInClause::IN,
+ _ => return Ok(None),
+ };
+
+ let (parent_type, parent_name) = match self.parse_one_of_keywords(&[
+ Keyword::ACCOUNT,
+ Keyword::DATABASE,
+ Keyword::SCHEMA,
+ Keyword::TABLE,
+ Keyword::VIEW,
+ ]) {
+ Some(Keyword::DATABASE) if self.peek_keywords(&[Keyword::STARTS,
Keyword::WITH]) => {
+ (Some(ShowStatementInParentType::Database), None)
+ }
+ Some(Keyword::SCHEMA) if self.peek_keywords(&[Keyword::STARTS,
Keyword::WITH]) => {
+ (Some(ShowStatementInParentType::Schema), None)
+ }
+ Some(parent_kw) => {
+ let parent_name = match self.parse_object_name(false) {
+ Ok(n) => Some(n),
+ _ => None,
+ };
+ match parent_kw {
+ Keyword::ACCOUNT =>
(Some(ShowStatementInParentType::Account), parent_name),
+ Keyword::DATABASE =>
(Some(ShowStatementInParentType::Database), parent_name),
+ Keyword::SCHEMA =>
(Some(ShowStatementInParentType::Schema), parent_name),
+ Keyword::TABLE => (Some(ShowStatementInParentType::Table),
parent_name),
+ Keyword::VIEW => (Some(ShowStatementInParentType::View),
parent_name),
+ _ => unreachable!(),
+ }
+ }
+ None => {
+ // Parsing MySQL style FROM tbl_name FROM db_name
+ // which is equivalent to FROM tbl_name.db_name
+ let mut parent_name = self.parse_object_name(false)?;
+ if self
+ .parse_one_of_keywords(&[Keyword::FROM, Keyword::IN])
+ .is_some()
+ {
+ parent_name.0.insert(0, self.parse_identifier(false)?);
Review Comment:
what is the intent with the insert operation (tried to diff against the
previous version but didn't see similar behavior)?
##########
src/parser/mod.rs:
##########
@@ -12315,6 +12310,124 @@ impl<'a> Parser<'a> {
}
false
}
+
+ /// Look for all of the expected keywords in sequence, without consuming
them
+ fn peek_keywords(&mut self, expected: &[Keyword]) -> bool {
+ let index = self.index;
+ for kw in expected {
+ if !self.parse_keyword(*kw) {
+ self.index = index;
+ return false;
+ }
+ }
+ self.index = index;
+ true
+ }
+
+ fn parse_show_stmt_options(&mut self) -> Result<ShowStatementOptions,
ParserError> {
+ let show_in;
+ let mut filter_position = None;
+ if self.dialect.supports_show_like_before_in() {
+ if let Some(filter) = self.parse_show_statement_filter()? {
+ filter_position =
Some(ShowStatementFilterPosition::Infix(filter));
+ }
+ show_in = self.maybe_parse_show_stmt_in()?;
+ } else {
+ show_in = self.maybe_parse_show_stmt_in()?;
+ if let Some(filter) = self.parse_show_statement_filter()? {
+ filter_position =
Some(ShowStatementFilterPosition::Suffix(filter));
+ }
+ }
+ let starts_with = self.maybe_parse_show_stmt_starts_with()?;
+ let limit = self.maybe_parse_show_stmt_limit()?;
+ let from = self.maybe_parse_show_stmt_from()?;
+ Ok(ShowStatementOptions {
+ filter_position,
+ show_in,
+ starts_with,
+ limit,
+ limit_from: from,
+ })
+ }
+
+ fn maybe_parse_show_stmt_in(&mut self) -> Result<Option<ShowStatementIn>,
ParserError> {
+ let clause = match self.parse_one_of_keywords(&[Keyword::FROM,
Keyword::IN]) {
+ Some(Keyword::FROM) => ShowStatementInClause::FROM,
+ Some(Keyword::IN) => ShowStatementInClause::IN,
+ _ => return Ok(None),
+ };
+
+ let (parent_type, parent_name) = match self.parse_one_of_keywords(&[
+ Keyword::ACCOUNT,
+ Keyword::DATABASE,
+ Keyword::SCHEMA,
+ Keyword::TABLE,
+ Keyword::VIEW,
+ ]) {
+ Some(Keyword::DATABASE) if self.peek_keywords(&[Keyword::STARTS,
Keyword::WITH]) => {
+ (Some(ShowStatementInParentType::Database), None)
+ }
+ Some(Keyword::SCHEMA) if self.peek_keywords(&[Keyword::STARTS,
Keyword::WITH]) => {
+ (Some(ShowStatementInParentType::Schema), None)
+ }
+ Some(parent_kw) => {
+ let parent_name = match self.parse_object_name(false) {
+ Ok(n) => Some(n),
+ _ => None,
Review Comment:
ah this one is ignoring an actual error that we would want to instead
propagate or?
##########
src/ast/mod.rs:
##########
@@ -7357,6 +7354,99 @@ impl Display for UtilityOption {
}
}
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+pub struct ShowStatementOptions {
Review Comment:
For the added structs/enums, since they're public can we add a short
description mentioning what they are? we could probably also link to one of the
snowflake show docs to serve as a generic reference for someone that comes
across it
##########
src/parser/mod.rs:
##########
@@ -12315,6 +12310,124 @@ impl<'a> Parser<'a> {
}
false
}
+
+ /// Look for all of the expected keywords in sequence, without consuming
them
+ fn peek_keywords(&mut self, expected: &[Keyword]) -> bool {
+ let index = self.index;
+ for kw in expected {
+ if !self.parse_keyword(*kw) {
+ self.index = index;
+ return false;
+ }
+ }
+ self.index = index;
+ true
Review Comment:
```suggestion
let matched = self.parse_keywords(expected);
self.index = index;
matched
```
thinking we could simplify this by reusing the existing parse_keywords
functionality?
Also can we move this method to the same area of the code as we have the
other `peek/parse_keyword*`? so that its easier to find when folks look for
that type of functionality
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
