-
Notifications
You must be signed in to change notification settings - Fork 684
GenericDialect: support colon operator for JsonAccess #2124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -17972,3 +17972,126 @@ fn parse_select_parenthesized_wildcard() { | |||
| assert_eq!(select2.projection.len(), 1); | ||||
| assert!(matches!(select2.projection[0], SelectItem::Wildcard(_))); | ||||
| } | ||||
|
|
||||
| // https://docs.snowflake.com/en/user-guide/querying-semistructured | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| #[test] | ||||
| fn parse_semi_structured_data_traversal() { | ||||
| let dialects = TestedDialects::new(vec![ | ||||
| Box::new(GenericDialect {}), | ||||
| Box::new(SnowflakeDialect {}), | ||||
| ]); | ||||
|
Comment on lines
+17979
to
+17982
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be all dialects instead? we don't seem to have special handling for generic and snowflake
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We actually do have special handling for datafusion-sqlparser-rs/src/parser/mod.rs Line 3814 in ce74e7f
(this is in main currently, not a change in this PR) From what I know, only Snowflake and Databricks support this syntax. So I can expand the test to include Databricks as well. But I don't think it would make sense to run this test on dialects that don't support this syntax. What do you think? |
||||
|
|
||||
| // most basic case | ||||
| let sql = "SELECT a:b FROM t"; | ||||
| let select = dialects.verified_only_select(sql); | ||||
| assert_eq!( | ||||
| SelectItem::UnnamedExpr(Expr::JsonAccess { | ||||
| value: Box::new(Expr::Identifier(Ident::new("a"))), | ||||
| path: JsonPath { | ||||
| path: vec![JsonPathElem::Dot { | ||||
| key: "b".to_owned(), | ||||
| quoted: false | ||||
| }] | ||||
| }, | ||||
| }), | ||||
| select.projection[0] | ||||
| ); | ||||
|
|
||||
| // identifier can be quoted | ||||
| let sql = r#"SELECT a:"my long object key name" FROM t"#; | ||||
| let select = dialects.verified_only_select(sql); | ||||
| assert_eq!( | ||||
| SelectItem::UnnamedExpr(Expr::JsonAccess { | ||||
| value: Box::new(Expr::Identifier(Ident::new("a"))), | ||||
| path: JsonPath { | ||||
| path: vec![JsonPathElem::Dot { | ||||
| key: "my long object key name".to_owned(), | ||||
| quoted: true | ||||
| }] | ||||
| }, | ||||
| }), | ||||
| select.projection[0] | ||||
| ); | ||||
|
|
||||
| dialects.verified_stmt("SELECT a:b::INT FROM t"); | ||||
|
|
||||
| // unquoted keywords are permitted in the object key | ||||
| let sql = "SELECT a:select, a:from FROM t"; | ||||
| let select = dialects.verified_only_select(sql); | ||||
| assert_eq!( | ||||
| vec![ | ||||
| SelectItem::UnnamedExpr(Expr::JsonAccess { | ||||
| value: Box::new(Expr::Identifier(Ident::new("a"))), | ||||
| path: JsonPath { | ||||
| path: vec![JsonPathElem::Dot { | ||||
| key: "select".to_owned(), | ||||
| quoted: false | ||||
| }] | ||||
| }, | ||||
| }), | ||||
| SelectItem::UnnamedExpr(Expr::JsonAccess { | ||||
| value: Box::new(Expr::Identifier(Ident::new("a"))), | ||||
| path: JsonPath { | ||||
| path: vec![JsonPathElem::Dot { | ||||
| key: "from".to_owned(), | ||||
| quoted: false | ||||
| }] | ||||
| }, | ||||
| }) | ||||
| ], | ||||
| select.projection | ||||
| ); | ||||
|
|
||||
| // multiple levels can be traversed | ||||
| // https://docs.snowflake.com/en/user-guide/querying-semistructured#dot-notation | ||||
| let sql = r#"SELECT a:foo."bar".baz"#; | ||||
| let select = dialects.verified_only_select(sql); | ||||
| assert_eq!( | ||||
| vec![SelectItem::UnnamedExpr(Expr::JsonAccess { | ||||
| value: Box::new(Expr::Identifier(Ident::new("a"))), | ||||
| path: JsonPath { | ||||
| path: vec![ | ||||
| JsonPathElem::Dot { | ||||
| key: "foo".to_owned(), | ||||
| quoted: false, | ||||
| }, | ||||
| JsonPathElem::Dot { | ||||
| key: "bar".to_owned(), | ||||
| quoted: true, | ||||
| }, | ||||
| JsonPathElem::Dot { | ||||
| key: "baz".to_owned(), | ||||
| quoted: false, | ||||
| } | ||||
| ] | ||||
| }, | ||||
| })], | ||||
| select.projection | ||||
| ); | ||||
|
|
||||
| // dot and bracket notation can be mixed (starting with : case) | ||||
| // https://docs.snowflake.com/en/user-guide/querying-semistructured#dot-notation | ||||
| let sql = r#"SELECT a:foo[0].bar"#; | ||||
| let select = dialects.verified_only_select(sql); | ||||
| assert_eq!( | ||||
| vec![SelectItem::UnnamedExpr(Expr::JsonAccess { | ||||
| value: Box::new(Expr::Identifier(Ident::new("a"))), | ||||
| path: JsonPath { | ||||
| path: vec![ | ||||
| JsonPathElem::Dot { | ||||
| key: "foo".to_owned(), | ||||
| quoted: false, | ||||
| }, | ||||
| JsonPathElem::Bracket { | ||||
| key: Expr::value(number("0")), | ||||
| }, | ||||
| JsonPathElem::Dot { | ||||
| key: "bar".to_owned(), | ||||
| quoted: false, | ||||
| } | ||||
| ] | ||||
| }, | ||||
| })], | ||||
| select.projection | ||||
| ); | ||||
| } | ||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the changes to the subscript behavior, we don't seem to have any new tests to accompany them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes actually came from a failing test, but I will add some more tests to explicitly look for this behavior.