iffyio commented on code in PR #1929: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1929#discussion_r2192824402
########## src/parser/mod.rs: ########## @@ -10353,70 +10353,84 @@ impl<'a> Parser<'a> { } } - /// Parse a possibly qualified, possibly quoted identifier, optionally allowing for wildcards, + /// Parse a possibly qualified, possibly quoted identifier, e.g. + /// `foo` or `myschema."table" + /// + /// The `in_table_clause` parameter indicates whether the object name is a table in a FROM, JOIN, + /// or similar table clause. Currently, this is used only to support unquoted hyphenated identifiers + /// in this context on BigQuery. + pub fn parse_object_name(&mut self, in_table_clause: bool) -> Result<ObjectName, ParserError> { + self.parse_object_name_inner(in_table_clause, false) + } + + /// Parse a possibly qualified, possibly quoted identifier, e.g. + /// `foo` or `myschema."table" + /// + /// The `in_table_clause` parameter indicates whether the object name is a table in a FROM, JOIN, + /// or similar table clause. Currently, this is used only to support unquoted hyphenated identifiers + /// in this context on BigQuery. + /// + /// The `allow_wildcards` parameter indicates whether to allow for wildcards in the object name /// e.g. *, *.*, `foo`.*, or "foo"."bar" - fn parse_object_name_with_wildcards( + fn parse_object_name_inner( &mut self, in_table_clause: bool, allow_wildcards: bool, ) -> Result<ObjectName, ParserError> { - let mut idents = vec![]; - + let mut parts = vec![]; if dialect_of!(self is BigQueryDialect) && in_table_clause { loop { let (ident, end_with_period) = self.parse_unquoted_hyphenated_identifier()?; - idents.push(ident); + parts.push(ObjectNamePart::Identifier(ident)); if !self.consume_token(&Token::Period) && !end_with_period { break; } } } else { loop { - let ident = if allow_wildcards && self.peek_token().token == Token::Mul { + if allow_wildcards && self.peek_token().token == Token::Mul { let span = self.next_token().span; - Ident { + parts.push(ObjectNamePart::Identifier(Ident { value: Token::Mul.to_string(), quote_style: None, span, + })); + } else if let Some(func_part) = + self.maybe_parse(|parser| parser.parse_object_name_function_part())? Review Comment: wondering can we instead extende the `else` clause to do something like this? ```rust else { let ident = self.parse_identifier()?; let part = if self.dialect.is_identifier_generating_function_name(&ident) { self.expect_token(&Token::LParen)?; let args = ... self.expect_token(&Token::RParen)?; ObjectNamePartFunction { name:ident, args } } else { ObjectNamePart::Identifier(ident)); } } ``` thinking that way we skip this extra branch, and the extra error scenario in the function part parsing ########## tests/sqlparser_snowflake.rs: ########## @@ -4232,3 +4232,122 @@ fn test_snowflake_create_view_with_composite_policy_name() { r#"CREATE VIEW X (COL WITH MASKING POLICY foo.bar.baz) AS SELECT * FROM Y"#; snowflake().verified_stmt(create_view_with_tag); } + +#[test] Review Comment: Could we add a test scenario for how this interacts with the [double dot](https://github.com/apache/datafusion-sqlparser-rs/blob/018992b22a78586dc4f176e80c6f1bf171bbfd7d/tests/sqlparser_snowflake.rs#L3330) feature? -- 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