iffyio commented on code in PR #1896: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1896#discussion_r2160244131
########## src/parser/mod.rs: ########## @@ -3156,18 +3156,21 @@ impl<'a> Parser<'a> { /// ``` /// /// [dictionary]: https://duckdb.org/docs/sql/data_types/struct#creating-structs - fn parse_duckdb_struct_literal(&mut self) -> Result<Expr, ParserError> { + /// [map]: https://clickhouse.com/docs/operations/settings/settings#additional_table_filters + fn parse_duckdb_and_clickhouse_struct_literal(&mut self) -> Result<Expr, ParserError> { Review Comment: Can we maybe rename this method to `parse_dictionary` and `parse_dictionary_field` respectively? so that the name doesn't change as a result of dialects being extended to support it ########## src/parser/mod.rs: ########## @@ -11190,7 +11194,12 @@ impl<'a> Parser<'a> { let key_values = self.parse_comma_separated(|p| { let key = p.parse_identifier()?; p.expect_token(&Token::Eq)?; - let value = p.parse_value()?.value; + + let value = if p.peek_token_ref().token == Token::LBrace { + p.parse_duckdb_and_clickhouse_struct_literal()? + } else { + Expr::Value(p.parse_value()?) + }; Review Comment: I think here it would be fine to parse an arbitrary expression instead, in order to simplify the logic, and likely to have comprehensive support covering other scenarios ```rust let value = self.parse_expr() ``` -- 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