iffyio commented on code in PR #1969: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1969#discussion_r2239372762
########## tests/sqlparser_common.rs: ########## @@ -11022,6 +11022,67 @@ fn parse_unpivot_table() { verified_stmt(sql_unpivot_include_nulls).to_string(), sql_unpivot_include_nulls ); + + let sql_unpivot_with_alias = concat!( + "SELECT * FROM sales AS s ", + "UNPIVOT INCLUDE NULLS (quantity FOR quarter IN (Q1 AS Quater1, Q2 AS Quater2, Q3 AS Quater3, Q4 AS Quater4)) AS u (product, quarter, quantity)" + ); Review Comment: it look like the formatting is a bit off here with cargo fmt, could you manually unindent these lines? ########## src/ast/query.rs: ########## @@ -1351,9 +1392,9 @@ pub enum TableFactor { /// See <https://docs.snowflake.com/en/sql-reference/constructs/unpivot>. Unpivot { table: Box<TableFactor>, - value: Ident, + value: Vec<Ident>, Review Comment: Hmm yeah I think repr wise we could represent this as an arbitrary `Expr`, it would cover both the single_value and multi_value scenarios. Then similarly, we change to `columns: Vec<ExprWithAlias>`, which would additionally cover the [fully qualified `column_name` variant](https://docs.databricks.com/aws/en/sql/language-manual/sql-ref-syntax-qry-select-unpivot#parameters) ########## tests/sqlparser_common.rs: ########## @@ -11022,6 +11022,67 @@ fn parse_unpivot_table() { verified_stmt(sql_unpivot_include_nulls).to_string(), sql_unpivot_include_nulls ); + + let sql_unpivot_with_alias = concat!( + "SELECT * FROM sales AS s ", + "UNPIVOT INCLUDE NULLS (quantity FOR quarter IN (Q1 AS Quater1, Q2 AS Quater2, Q3 AS Quater3, Q4 AS Quater4)) AS u (product, quarter, quantity)" + ); + + if let Unpivot { value, columns, .. } = + &verified_only_select(sql_unpivot_with_alias).from[0].relation + { + assert_eq!( + *columns, + vec![ + IdentsWithAlias::new(vec![Ident::new("Q1")], Some(Ident::new("Quater1"))), + IdentsWithAlias::new(vec![Ident::new("Q2")], Some(Ident::new("Quater2"))), + IdentsWithAlias::new(vec![Ident::new("Q3")], Some(Ident::new("Quater3"))), + IdentsWithAlias::new(vec![Ident::new("Q4")], Some(Ident::new("Quater4"))), + ] + ); + assert_eq!(*value, vec![Ident::new("quantity")]); + } + + assert_eq!( + verified_stmt(sql_unpivot_with_alias).to_string(), + sql_unpivot_with_alias + ); + + let sql_unpivot_with_alias = concat!( Review Comment: this has the same name as the preceeding `sql_unpivot_with_alias` scenario, but testing different aspects, do we need to rename this accordingly? -- 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