iffyio commented on code in PR #2375:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2375#discussion_r3577054005


##########
src/parser/mod.rs:
##########
@@ -16575,6 +16581,10 @@ impl<'a> Parser<'a> {
                 with_offset_alias,
                 with_ordinality,
             })
+        } else if self.dialect.supports_unpivot_expr_in_from()
+            && self.parse_keyword(Keyword::UNPIVOT)

Review Comment:
   can we change this to `self.peek(UNPIVOT)` so that the 
`parse_unpivot_expr_table_factor` is standalone since we're making it a public 
function?



##########
src/dialect/mod.rs:
##########
@@ -1267,6 +1267,16 @@ pub trait Dialect: Debug + Any {
         false
     }
 
+    /// Returns true if the dialect supports object-unpivot table factors in 
the FROM clause.
+    ///
+    /// Syntax:
+    /// ```sql
+    /// UNPIVOT expression AS value_alias [AT attribute_alias]
+    /// ```
+    fn supports_unpivot_expr_in_from(&self) -> bool {

Review Comment:
   ```suggestion
       fn supports_unpivot_expr(&self) -> bool {
   ```



##########
src/dialect/redshift.rs:
##########
@@ -113,6 +113,10 @@ impl Dialect for RedshiftSqlDialect {
         true
     }
 
+    fn supports_unpivot_expr_in_from(&self) -> bool {

Review Comment:
   can we add a link to the redshift docs?



##########
tests/sqlparser_redshift.rs:
##########
@@ -542,3 +542,21 @@ fn test_partiql_from_alias_with_at_index() {
         _ => panic!("expected table factor"),
     }
 }
+
+#[test]
+fn parse_unpivot_expression() {
+    let sql = r#"SELECT t.id, k, v FROM test_colors as t, UNPIVOT 
t.count_by_color AS v AT k;
+"#;
+
+    redshift().parse_sql_statements(sql).unwrap();
+
+}
+
+#[test]
+fn parse_unpivot_no_brackets() {
+    let sql = r#"SELECT t.id, k, v FROM test_colors as t, UNPIVOT t AS v AT k;
+"#;
+
+    redshift().parse_sql_statements(sql).unwrap();
+

Review Comment:
   let's use verified_stmt also we can merge the test cases into the same 
function



##########
src/ast/query.rs:
##########
@@ -1653,6 +1653,20 @@ pub enum TableFactor {
         /// Optional alias for the resulting table.
         alias: Option<TableAlias>,
     },
+    /// Object unpivoting on a SUPER expression in the FROM clause.
+    ///
+    /// Syntax:
+    /// ```sql
+    /// UNPIVOT expression AS value_alias [AT attribute_alias]
+    /// ```

Review Comment:
   Can we add a link to the docs describing the syntax?



-- 
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]

Reply via email to