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-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
