Added support for parsing nested object in query for Snowflake - #2359
Added support for parsing nested object in query for Snowflake#2359kfirSatori wants to merge 6 commits into
Conversation
|
@iffyio Can you please help resolve the PR? |
| let fields = self.parse_union_type_def()?; | ||
| Ok(DataType::Union(fields)) | ||
| } | ||
| Keyword::OBJECT if dialect_is!(dialect is SnowflakeDialect | GenericDialect) => { |
There was a problem hiding this comment.
can we change this to use a dialect method?
There was a problem hiding this comment.
Hi @iffyio, i dont understand what do you mean.
i did it the same way done as UNION and NULLABLE
There was a problem hiding this comment.
ah so I meant to use instead a self.dialect.supportsxxx() style method - that's preferred over the dialect_is macro
| self.expect_keyword_is(Keyword::OBJECT)?; | ||
| // Object type may have no fields: OBJECT or OBJECT() | ||
| if !self.peek_token_ref().token.eq(&Token::LParen) { | ||
| Ok(DataType::Object(vec![])) | ||
| } else { | ||
| self.expect_token(&Token::LParen)?; | ||
| let fields = if self.peek_token_ref().token == Token::RParen { | ||
| vec![] | ||
| } else { | ||
| self.parse_comma_separated(|parser| { | ||
| let field_name = parser.parse_identifier()?; | ||
| let field_type = parser.parse_data_type()?; | ||
| Ok(StructField { | ||
| field_name: Some(field_name), | ||
| field_type, | ||
| options: None, | ||
| }) | ||
| })? | ||
| }; | ||
| self.expect_token(&Token::RParen)?; |
There was a problem hiding this comment.
can we move this to its own parse_object_data_type() or similar helper method?
| field VARCHAR | ||
| )));"#; | ||
|
|
||
| snowflake().parse_sql_statements(sql).unwrap(); |
There was a problem hiding this comment.
can we use verified_stmt? also if we can include test cases for mulltiple struct fields, zero struct fields, OBJECT() and OBJECT
| /// Object type, see [Snowflake]. | ||
| /// | ||
| /// [Snowflake]: https://docs.snowflake.com/en/sql-reference/data-types-semistructured#object | ||
| Object(Vec<StructField>), |
There was a problem hiding this comment.
Can we turn this into a Object { xxx: Option<Vec<StructField>> }? the named field syntax makes it possible to extend later on (like attaching spans) and the option iiuc is needed if we're to properly support OBJECT syntax (unclear since the behavior suggests it but the tests don't cover it)
| let fields = self.parse_union_type_def()?; | ||
| Ok(DataType::Union(fields)) | ||
| } | ||
| Keyword::OBJECT if dialect_is!(dialect is SnowflakeDialect | GenericDialect) => { |
There was a problem hiding this comment.
ah so I meant to use instead a self.dialect.supportsxxx() style method - that's preferred over the dialect_is macro
| #[test] | ||
| fn parse_nested_object() { | ||
| // nested OBJECT with a single field | ||
| snowflake().verified_stmt("SELECT TRY_CAST(PARSE_JSON('{\"obj_field\":{\"field\":\"value\",}}') AS OBJECT(obj_field OBJECT(field VARCHAR)))"); |
There was a problem hiding this comment.
heads up after switching to dialect method, these tests can use all_dialects_where(|d| d.supportsxxx()) to select the dialect instead of hardcoding snowflake only
| /// ``` | ||
| /// | ||
| /// [1]: https://duckdb.org/docs/sql/data_types/union.html | ||
| fn parse_object_data_type(&mut self) -> Result<DataType, ParserError> { |
There was a problem hiding this comment.
hmm this diff looks a bit off,does the comment above apply to this new function? it looks like it was rather meant for parse_union_type_def
| /// | ||
| /// [DuckDB]: https://duckdb.org/docs/sql/data_types/union.html | ||
| Union(Vec<UnionField>), | ||
| /// Object type, see [Snowflake]. |
There was a problem hiding this comment.
| /// Object type, see [Snowflake]. | |
| /// Object type. |
2 participants
Queries parsing like:
SELECT TRY_CAST(PARSE_JSON('{"obj_field":{"field":"value",}}') AS OBJECT(obj_field OBJECT( field VARCHAR)));in Snowflake are failing since nested object are not supported.
I have added support for it.