iffyio commented on code in PR #2009:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2009#discussion_r2290195395
##########
src/parser/mod.rs:
##########
@@ -13769,6 +13774,66 @@ impl<'a> Parser<'a> {
Ok(XmlPassingClause { arguments })
}
+ fn parse_semantic_view_table_factor(&mut self) -> Result<TableFactor,
ParserError> {
Review Comment:
```suggestion
/// Parse a [TableFactor::SemanticView]
fn parse_semantic_view_table_factor(&mut self) -> Result<TableFactor,
ParserError> {
```
##########
tests/sqlparser_snowflake.rs:
##########
@@ -4581,3 +4581,129 @@ fn test_drop_constraints() {
snowflake().verified_stmt("ALTER TABLE tbl DROP FOREIGN KEY k1 RESTRICT");
snowflake().verified_stmt("ALTER TABLE tbl DROP CONSTRAINT c1 CASCADE");
}
+
+#[test]
+fn test_semantic_view_all_variants_should_pass() {
+ let test_cases = [
+ ("SELECT * FROM SEMANTIC_VIEW(model)", None),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1, dim2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS met1, met2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model FACTS fact1, fact2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 METRICS met1)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 WHERE x > 0)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1) AS sv",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS DATE_PART('year',
col))",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS orders.col,
orders.col2)",
+ None,
+ ),
+ // We can parse in any order bu will always produce a result in a
fixed order.
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model WHERE x > 0 DIMENSIONS dim1)",
+ Some("SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 WHERE x >
0)"),
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS met1 DIMENSIONS dim1)",
+ Some("SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 METRICS
met1)"),
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model FACTS fact1 DIMENSIONS dim1)",
+ Some("SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 FACTS
fact1)"),
+ ),
+ ];
+
+ for (input_sql, expected_sql) in test_cases {
+ if let Some(expected) = expected_sql {
+ // Test that non-canonical order gets normalized
+ let parsed = snowflake().parse_sql_statements(input_sql).unwrap();
+ let formatted = parsed[0].to_string();
+ assert_eq!(formatted, expected);
+ } else {
+ snowflake().verified_stmt(input_sql);
+ }
+ }
+}
+
+#[test]
+fn test_semantic_view_invalid_queries_should_fail() {
+ let invalid_sqls = [
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 INVALID inv1)",
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 DIMENSIONS dim2)",
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS SUM(met1.avg))",
+ ];
+
+ for sql in invalid_sqls {
+ let result = snowflake().parse_sql_statements(sql);
+ assert!(result.is_err(), "Expected error for invalid SQL: {}", sql);
+ }
+}
+
+#[test]
+fn test_semantic_view_ast_structure() {
Review Comment:
can we inline all three scenarios into the same
`test_semantic_view_table_factor()` function? since they belong to the same
feature
##########
tests/sqlparser_snowflake.rs:
##########
@@ -4581,3 +4581,129 @@ fn test_drop_constraints() {
snowflake().verified_stmt("ALTER TABLE tbl DROP FOREIGN KEY k1 RESTRICT");
snowflake().verified_stmt("ALTER TABLE tbl DROP CONSTRAINT c1 CASCADE");
}
+
+#[test]
+fn test_semantic_view_all_variants_should_pass() {
+ let test_cases = [
+ ("SELECT * FROM SEMANTIC_VIEW(model)", None),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1, dim2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS met1, met2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model FACTS fact1, fact2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 METRICS met1)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 WHERE x > 0)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1) AS sv",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS DATE_PART('year',
col))",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS orders.col,
orders.col2)",
+ None,
+ ),
+ // We can parse in any order bu will always produce a result in a
fixed order.
Review Comment:
```suggestion
// We can parse in any order but will always produce a result in a
fixed order.
```
##########
src/parser/mod.rs:
##########
@@ -13438,6 +13439,10 @@ impl<'a> Parser<'a> {
} else if self.parse_keyword_with_tokens(Keyword::XMLTABLE,
&[Token::LParen]) {
self.prev_token();
self.parse_xml_table_factor()
+ } else if self.dialect.supports_semantic_view()
+ && self.parse_keyword_with_tokens(Keyword::SEMANTIC_VIEW,
&[Token::LParen])
+ {
+ self.parse_semantic_view_table_factor()
Review Comment:
It would be good to have `parse_semantic_view_table_factor()` as a
standalone function that can parse a semantic view table - so that it can be
used in other contexts. I think one way to do that would be to call
`self.prev_token()` twice here but likely a better way would be to implement a
similar `self.peek_keyword_with_tokens(Keyword::SEMANTIC_VIEW,
&[Token::LParen])` that we can call here?
##########
tests/sqlparser_snowflake.rs:
##########
@@ -4581,3 +4581,129 @@ fn test_drop_constraints() {
snowflake().verified_stmt("ALTER TABLE tbl DROP FOREIGN KEY k1 RESTRICT");
snowflake().verified_stmt("ALTER TABLE tbl DROP CONSTRAINT c1 CASCADE");
}
+
+#[test]
+fn test_semantic_view_all_variants_should_pass() {
+ let test_cases = [
+ ("SELECT * FROM SEMANTIC_VIEW(model)", None),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1, dim2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS met1, met2)",
Review Comment:
```suggestion
"SELECT * FROM SEMANTIC_VIEW(a.b METRICS c.d, met2)",
```
thinking we can include a test case that parses a qualified object name
instead vs identifier
##########
tests/sqlparser_snowflake.rs:
##########
@@ -4581,3 +4581,129 @@ fn test_drop_constraints() {
snowflake().verified_stmt("ALTER TABLE tbl DROP FOREIGN KEY k1 RESTRICT");
snowflake().verified_stmt("ALTER TABLE tbl DROP CONSTRAINT c1 CASCADE");
}
+
+#[test]
+fn test_semantic_view_all_variants_should_pass() {
+ let test_cases = [
+ ("SELECT * FROM SEMANTIC_VIEW(model)", None),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1, dim2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model METRICS met1, met2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model FACTS fact1, fact2)",
+ None,
+ ),
+ (
+ "SELECT * FROM SEMANTIC_VIEW(model DIMENSIONS dim1 METRICS met1)",
+ None,
Review Comment:
Can we include test to demonstrate that facts can accept arbitrary
expressions in addition to identifiers?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
