Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
alamb commented on PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#issuecomment-2500876679 We think this may have introduced a small regression: - https://github.com/apache/datafusion-sqlparser-rs/issues/1554 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
alamb merged PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
alamb commented on PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#issuecomment-2473397821 🚀 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
alamb commented on PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#issuecomment-2473400588 Thanks again @iffyio and @yoavcloud -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
iffyio commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1838531561 ## src/ast/mod.rs: ## @@ -7357,6 +7354,102 @@ impl Display for UtilityOption { } } +/// Represents the different options available for a SHOW +/// statement to filter the results. Example from Snowflake: +/// https://docs.snowflake.com/en/sql-reference/sql/show-tables +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +pub struct ShowStatementOptions { Review Comment: Oh one thing actually, we might be missing the serde and visit derive attributes on some of the new structs? -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
iffyio commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1838521802 ## 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 { +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, 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: Ah I see, that's interesting ## src/dialect/mod.rs: ## @@ -606,6 +606,12 @@ pub trait Dialect: Debug + Any { fn supports_top_before_distinct(&self) -> bool { false } + +/// Returns true if this dialect support the `LIKE 'pattern'` option in Review Comment: ```suggestion /// Returns true if this dialect supports the `LIKE 'pattern'` option in ``` ## 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 '%
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1835688630 ## 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: Please see test_show_dbs_schemas_tables_views in sqlparser_common. Perhaps I should remove the non-Snowflake specific tests from this file. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1835682251 ## 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 { +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, 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: @iffyio actually, the issue here is that the optional parent name is located at the end of the statement. We need to differentiate between SHOW TABLES IN DATABASE and SHOW TABLES IN DATABASE db1. The failure to parse the object name is the indication. Do you have a suggestion as to how to handle this case better? -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1835676890 ## 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 { +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, 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: See here for the previous code: https://github.com/apache/datafusion-sqlparser-rs/blob/334a5bf354ac964a14f2e2dc3851ed4d853b28d7/src/parser/mod.rs#L9683 It basically transforms the SHOW COLUMNS FROM tbl1 FROM db1 to the canonical form SHOW COLUMNS FROM db1.tbl1 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1835675842 ## 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 { +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, 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: Added another condition above to ensure we can expect an object name here -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
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 { +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, 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, +pub starts_with: Option, +pub limit: Option, +pub limit_from: Option, +pub filter_position: Option, +} + +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 DATAB
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud commented on PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#issuecomment-2466359156 @iffyio thanks for the great comments, it really helped me improve this code. New commit for you to look at. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1835316915 ## src/parser/mod.rs: ## @@ -9685,58 +9711,95 @@ impl<'a> Parser<'a> { extended: bool, full: bool, ) -> Result { -self.expect_one_of_keywords(&[Keyword::FROM, Keyword::IN])?; -let object_name = self.parse_object_name(false)?; -let table_name = match self.parse_one_of_keywords(&[Keyword::FROM, Keyword::IN]) { -Some(_) => { -let db_name = vec![self.parse_identifier(false)?]; -let ObjectName(table_name) = object_name; -let object_name = db_name.into_iter().chain(table_name).collect(); -ObjectName(object_name) -} -None => object_name, -}; -let filter = self.parse_show_statement_filter()?; +let filter; +let filter_position; +let show_in; +if self.dialect.supports_show_like_before_in() { +filter = self.parse_show_statement_filter()?; +filter_position = ShowStatementFilterPosition::InTheMiddle; +show_in = self.parse_show_opt_in()?; +} else { +show_in = self.parse_show_opt_in()?; +filter = self.parse_show_statement_filter()?; +filter_position = ShowStatementFilterPosition::AtTheEnd; +} Ok(Statement::ShowColumns { extended, full, -table_name, +show_in, filter, +filter_position, }) } -pub fn parse_show_tables( +fn parse_show_tables( &mut self, +terse: bool, extended: bool, full: bool, +external: bool, ) -> Result { -let (clause, db_name) = match self.parse_one_of_keywords(&[Keyword::FROM, Keyword::IN]) { -Some(Keyword::FROM) => (Some(ShowClause::FROM), Some(self.parse_identifier(false)?)), -Some(Keyword::IN) => (Some(ShowClause::IN), Some(self.parse_identifier(false)?)), -_ => (None, None), -}; -let filter = self.parse_show_statement_filter()?; +let history = !external && self.parse_keyword(Keyword::HISTORY); +let filter; +let show_in; +let filter_position; +if self.dialect.supports_show_like_before_in() { +filter = self.parse_show_statement_filter()?; +//YOAV: here we have a problem, the hint is DB-dependent (table in a schemas or some other object) Review Comment: Oops... fixed -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1835307304 ## 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, 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: I don't think we can get to an unexpected state here because we will only get a Some if the parser consumed a FROM or an IN keyword. If it's neither, it means this show option does not exist in the statement and no other tokens were consumed, so we should just return the None. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
Re: [PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
iffyio commented on code in PR #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501#discussion_r1833098300 ## src/parser/mod.rs: ## @@ -9685,58 +9711,95 @@ impl<'a> Parser<'a> { extended: bool, full: bool, ) -> Result { -self.expect_one_of_keywords(&[Keyword::FROM, Keyword::IN])?; -let object_name = self.parse_object_name(false)?; -let table_name = match self.parse_one_of_keywords(&[Keyword::FROM, Keyword::IN]) { -Some(_) => { -let db_name = vec![self.parse_identifier(false)?]; -let ObjectName(table_name) = object_name; -let object_name = db_name.into_iter().chain(table_name).collect(); -ObjectName(object_name) -} -None => object_name, -}; -let filter = self.parse_show_statement_filter()?; +let filter; +let filter_position; +let show_in; +if self.dialect.supports_show_like_before_in() { +filter = self.parse_show_statement_filter()?; +filter_position = ShowStatementFilterPosition::InTheMiddle; +show_in = self.parse_show_opt_in()?; +} else { +show_in = self.parse_show_opt_in()?; +filter = self.parse_show_statement_filter()?; +filter_position = ShowStatementFilterPosition::AtTheEnd; +} Ok(Statement::ShowColumns { extended, full, -table_name, +show_in, filter, +filter_position, }) } -pub fn parse_show_tables( +fn parse_show_tables( &mut self, +terse: bool, extended: bool, full: bool, +external: bool, ) -> Result { -let (clause, db_name) = match self.parse_one_of_keywords(&[Keyword::FROM, Keyword::IN]) { -Some(Keyword::FROM) => (Some(ShowClause::FROM), Some(self.parse_identifier(false)?)), -Some(Keyword::IN) => (Some(ShowClause::IN), Some(self.parse_identifier(false)?)), -_ => (None, None), -}; -let filter = self.parse_show_statement_filter()?; +let history = !external && self.parse_keyword(Keyword::HISTORY); +let filter; +let show_in; +let filter_position; +if self.dialect.supports_show_like_before_in() { +filter = self.parse_show_statement_filter()?; +//YOAV: here we have a problem, the hint is DB-dependent (table in a schemas or some other object) Review Comment: is the comment still valid or the issue was resolved? ## src/ast/mod.rs: ## @@ -4382,79 +4406,226 @@ impl fmt::Display for Statement { Statement::ShowColumns { extended, full, -table_name, +show_in, filter, +filter_position, } => { write!( f, -"SHOW {extended}{full}COLUMNS FROM {table_name}", +"SHOW {extended}{full}COLUMNS", extended = if *extended { "EXTENDED " } else { "" }, full = if *full { "FULL " } else { "" }, -table_name = table_name, )?; -if let Some(filter) = filter { -write!(f, " {filter}")?; +if filter_position == &ShowStatementFilterPosition::InTheMiddle { +if let Some(filter) = filter { +write!(f, " {filter}")?; +} +if let Some(show_in) = show_in { +write!(f, " {show_in}")?; +} +} +if filter_position == &ShowStatementFilterPosition::AtTheEnd { +if let Some(show_in) = show_in { +write!(f, " {show_in}")?; +} +if let Some(filter) = filter { +write!(f, " {filter}")?; +} } Ok(()) } -Statement::ShowDatabases { filter } => { -write!(f, "SHOW DATABASES")?; -if let Some(filter) = filter { -write!(f, " {filter}")?; -} +Statement::ShowDatabases { +terse, +history, +filter, +show_in, +starts_with, +limit, +from, +} => { +write!( +f, +"SHOW {terse}DATABASES", +terse = if *terse { "TERSE " } else { "" } +)?; +write!( +f, +"{history}{filter}{show_in}{starts_with}{limit}{from}", +history = if *hist
[PR] Add support for Snowflake SHOW DATABASES/SCHEMAS/TABLES/VIEWS/COLUMNS statements [datafusion-sqlparser-rs]
yoavcloud opened a new pull request, #1501: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1501 This PR extends the existing support for SHOW statements to Snowflake -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org