You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "7phs (via GitHub)" <gi...@apache.org> on 2025/03/04 14:27:49 UTC
[PR] Snowflake: Support dollar quoted comment of table, view and field [datafusion-sqlparser-rs]
7phs opened a new pull request, #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755
Snowflake [supports](https://docs.snowflake.com/en/sql-reference/literals-table) dollar-quoted comments as string literals when creating tables, views, and their fields.
For example:
```
CREATE OR REPLACE TEMPORARY VIEW foo.bar.baz (
"COL_1" COMMENT $$comment 1$$
) COMMENT = $$view comment$$ AS (
SELECT 1
)
```
This pull request adds support for this type of comments.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comments [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comment when creating tables, views, and their fields [datafusion-sqlparser-rs]
Posted by "7phs (via GitHub)" <gi...@apache.org>.
7phs commented on code in PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755#discussion_r1994036934
##########
src/dialect/snowflake.rs:
##########
@@ -245,6 +245,14 @@ impl Dialect for SnowflakeDialect {
.map(|p| Some(ColumnOption::Policy(ColumnPolicy::ProjectionPolicy(p)))))
} else if parser.parse_keywords(&[Keyword::TAG]) {
Ok(parse_column_tags(parser, with).map(|p| Some(ColumnOption::Tags(p))))
+ } else if parser.parse_keywords(&[Keyword::COMMENT]) {
+ let next_token = parser.next_token();
+ match next_token.token {
+ Token::DollarQuotedString(value, ..) => {
+ Ok(Ok(Some(ColumnOption::Comment(value.value))))
+ }
+ _ => Err(ParserError::ParserError("not found match".to_string())),
Review Comment:
That makes sense. I have moved this code to the parser level.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comment when creating tables, views, and their fields [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755#discussion_r1995813212
##########
tests/sqlparser_snowflake.rs:
##########
@@ -976,6 +976,27 @@ fn parse_sf_create_or_replace_with_comment_for_snowflake() {
}
}
+#[test]
+fn parse_sf_create_table_or_view_with_dollar_quoted_comment() {
+ assert!(snowflake()
+ .parse_sql_statements(
+ r#"CREATE OR REPLACE TEMPORARY VIEW foo.bar.baz (
+ "COL_1" COMMENT $$comment 1$$
+ ) COMMENT = $$view comment$$ AS (
+ SELECT 1
+ )"#
+ )
+ .is_ok());
+
+ assert!(snowflake()
+ .parse_sql_statements(
+ r#"CREATE TABLE my_table (
+ a STRING COMMENT $$comment 1$$
+ ) COMMENT = $$table comment$$"#
+ )
Review Comment:
Ah that makes sense, I think we can leave as is in that case in order to limit the scope of the PR
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comment when creating tables, views, and their fields [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755#discussion_r1984480265
##########
src/parser/mod.rs:
##########
@@ -6922,18 +6924,23 @@ impl<'a> Parser<'a> {
})
}
- pub fn parse_optional_inline_comment(&mut self) -> Result<Option<CommentDef>, ParserError> {
+ pub fn parse_optional_inline_comment(
+ &mut self,
+ support_dollar_quoted_comment: bool,
+ ) -> Result<Option<CommentDef>, ParserError> {
let comment = if self.parse_keyword(Keyword::COMMENT) {
let has_eq = self.consume_token(&Token::Eq);
let next_token = self.next_token();
- match next_token.token {
- Token::SingleQuotedString(str) => Some(if has_eq {
- CommentDef::WithEq(str)
- } else {
- CommentDef::WithoutEq(str)
- }),
+ let comment = match next_token.token {
+ Token::SingleQuotedString(str) => str,
+ Token::DollarQuotedString(str) if support_dollar_quoted_comment => str.value,
Review Comment:
I think no need to guard this with a flag, we can always accept a dollar quoted string if one shows up. Also noticed we have similar [function here](https://github.com/apache/datafusion-sqlparser-rs/blob/24a2968b33787626fe63fc626c19dbfb50ec48f5/src/parser/mod.rs#L5323-L5328) could we maybe pull that part out into a helper and reuse in both places?
##########
tests/sqlparser_snowflake.rs:
##########
@@ -976,6 +976,27 @@ fn parse_sf_create_or_replace_with_comment_for_snowflake() {
}
}
+#[test]
+fn parse_sf_create_table_or_view_with_dollar_quoted_comment() {
+ assert!(snowflake()
+ .parse_sql_statements(
+ r#"CREATE OR REPLACE TEMPORARY VIEW foo.bar.baz (
+ "COL_1" COMMENT $$comment 1$$
+ ) COMMENT = $$view comment$$ AS (
+ SELECT 1
+ )"#
+ )
+ .is_ok());
+
+ assert!(snowflake()
+ .parse_sql_statements(
+ r#"CREATE TABLE my_table (
+ a STRING COMMENT $$comment 1$$
+ ) COMMENT = $$table comment$$"#
+ )
Review Comment:
can we use `snowflake().verified_stmt(sql)` for the tests?
##########
src/dialect/snowflake.rs:
##########
@@ -245,6 +245,14 @@ impl Dialect for SnowflakeDialect {
.map(|p| Some(ColumnOption::Policy(ColumnPolicy::ProjectionPolicy(p)))))
} else if parser.parse_keywords(&[Keyword::TAG]) {
Ok(parse_column_tags(parser, with).map(|p| Some(ColumnOption::Tags(p))))
+ } else if parser.parse_keywords(&[Keyword::COMMENT]) {
+ let next_token = parser.next_token();
+ match next_token.token {
+ Token::DollarQuotedString(value, ..) => {
+ Ok(Ok(Some(ColumnOption::Comment(value.value))))
+ }
+ _ => Err(ParserError::ParserError("not found match".to_string())),
Review Comment:
hmm the error message sounds like it'd be confusing what we mean by 'not found match'.
Left a comment below about reusing the parsing code, we probably want to reuse it here as well (any reason not to cover the single quote case 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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comments [datafusion-sqlparser-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755#issuecomment-2727720945
🚀
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comment when creating tables, views, and their fields [datafusion-sqlparser-rs]
Posted by "7phs (via GitHub)" <gi...@apache.org>.
7phs commented on code in PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755#discussion_r1994036120
##########
src/parser/mod.rs:
##########
@@ -6922,18 +6924,23 @@ impl<'a> Parser<'a> {
})
}
- pub fn parse_optional_inline_comment(&mut self) -> Result<Option<CommentDef>, ParserError> {
+ pub fn parse_optional_inline_comment(
+ &mut self,
+ support_dollar_quoted_comment: bool,
+ ) -> Result<Option<CommentDef>, ParserError> {
let comment = if self.parse_keyword(Keyword::COMMENT) {
let has_eq = self.consume_token(&Token::Eq);
let next_token = self.next_token();
- match next_token.token {
- Token::SingleQuotedString(str) => Some(if has_eq {
- CommentDef::WithEq(str)
- } else {
- CommentDef::WithoutEq(str)
- }),
+ let comment = match next_token.token {
+ Token::SingleQuotedString(str) => str,
+ Token::DollarQuotedString(str) if support_dollar_quoted_comment => str.value,
Review Comment:
A parameter `support_dollar_quoted_comment` is removed.
##########
tests/sqlparser_snowflake.rs:
##########
@@ -976,6 +976,27 @@ fn parse_sf_create_or_replace_with_comment_for_snowflake() {
}
}
+#[test]
+fn parse_sf_create_table_or_view_with_dollar_quoted_comment() {
+ assert!(snowflake()
+ .parse_sql_statements(
+ r#"CREATE OR REPLACE TEMPORARY VIEW foo.bar.baz (
+ "COL_1" COMMENT $$comment 1$$
+ ) COMMENT = $$view comment$$ AS (
+ SELECT 1
+ )"#
+ )
+ .is_ok());
+
+ assert!(snowflake()
+ .parse_sql_statements(
+ r#"CREATE TABLE my_table (
+ a STRING COMMENT $$comment 1$$
+ ) COMMENT = $$table comment$$"#
+ )
Review Comment:
1. There is no code for storing a comment quotation. Could you clarify whether it makes sense to update all the code that handles comment?
2. Snowflake converts a dollar-quoted comment into a single-quoted comment in the DDL representation of table/view creation.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comment when creating tables, views, and their fields [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755#discussion_r1995819665
##########
src/parser/mod.rs:
##########
@@ -6926,14 +6927,16 @@ impl<'a> Parser<'a> {
let comment = if self.parse_keyword(Keyword::COMMENT) {
let has_eq = self.consume_token(&Token::Eq);
let next_token = self.next_token();
- match next_token.token {
- Token::SingleQuotedString(str) => Some(if has_eq {
- CommentDef::WithEq(str)
- } else {
- CommentDef::WithoutEq(str)
- }),
+ let comment = match next_token.token {
+ Token::SingleQuotedString(str) => str,
+ Token::DollarQuotedString(str) => str.value,
_ => self.expected("comment", next_token)?,
- }
+ };
Review Comment:
can we move this part to be reusable? something like
```rust
fn parse_comment_value() -> Result<String> {
let value = match next_token.token {
Token::SingleQuotedString(str) => str,
Token::DollarQuotedString(str) => str.value,
_ => self.expected("comment", next_token)?,
};
Ok(value)
}
```
and then to call the function from both places
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Snowflake: Support dollar quoted comment when creating tables, views, and their fields [datafusion-sqlparser-rs]
Posted by "7phs (via GitHub)" <gi...@apache.org>.
7phs commented on code in PR #1755:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1755#discussion_r1995866489
##########
src/parser/mod.rs:
##########
@@ -6926,14 +6927,16 @@ impl<'a> Parser<'a> {
let comment = if self.parse_keyword(Keyword::COMMENT) {
let has_eq = self.consume_token(&Token::Eq);
let next_token = self.next_token();
- match next_token.token {
- Token::SingleQuotedString(str) => Some(if has_eq {
- CommentDef::WithEq(str)
- } else {
- CommentDef::WithoutEq(str)
- }),
+ let comment = match next_token.token {
+ Token::SingleQuotedString(str) => str,
+ Token::DollarQuotedString(str) => str.value,
_ => self.expected("comment", next_token)?,
- }
+ };
Review Comment:
Updated.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org