Added support for unpivot in Redshift with expression and bracketsless - #2375
Conversation
|
@iffyio Can you help me with the PR, it's a while here. |
| /// Syntax: | ||
| /// ```sql | ||
| /// UNPIVOT expression AS value_alias [AT attribute_alias] | ||
| /// ``` |
There was a problem hiding this comment.
Can we add a link to the docs describing the syntax?
| /// ```sql | ||
| /// UNPIVOT expression AS value_alias [AT attribute_alias] | ||
| /// ``` | ||
| fn supports_unpivot_expr_in_from(&self) -> bool { |
There was a problem hiding this comment.
| fn supports_unpivot_expr_in_from(&self) -> bool { | |
| fn supports_unpivot_expr(&self) -> bool { |
| 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(); | ||
|
|
There was a problem hiding this comment.
let's use verified_stmt also we can merge the test cases into the same function
| with_ordinality, | ||
| }) | ||
| } else if self.dialect.supports_unpivot_expr_in_from() | ||
| && self.parse_keyword(Keyword::UNPIVOT) |
There was a problem hiding this 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?
There was a problem hiding this comment.
I meant essentially that the condition becomes else if self.dialect.supports_unpivot_expr_in_from() && self.peek_keyword(Keyword::UNPIVOT) { parse_unpivot_expr_table_factor() } - then parse_unpivot_expr_table_factor is updated to expect the UNPIVOT keyword
There was a problem hiding this comment.
the parse_keyword still remains, the goal of introducing peek_keyword is that parse_keyword moves into the parse_unpivot_expr_table_factor function as an expect call instead
| true | ||
| } | ||
|
|
||
| fn supports_unpivot_expr_in_from(&self) -> bool { |
There was a problem hiding this comment.
can we add a link to the redshift docs?
There was a problem hiding this comment.
I have added the doc link on the top, here as well?
|
Hi @iffyio , I've fixed the comments, please tell me if there's anything else |
| /// | ||
| /// Syntax: | ||
| /// ```sql | ||
| /// UNPIVOT expression AS value_alias [AT attribute_alias] |
There was a problem hiding this comment.
| /// UNPIVOT expression AS value_alias [AT attribute_alias] | |
| /// SELECT * FROM T UNPIVOT expression AS value_alias [AT attribute_alias] |
| with_ordinality, | ||
| }) | ||
| } else if self.dialect.supports_unpivot_expr_in_from() | ||
| && self.parse_keyword(Keyword::UNPIVOT) |
There was a problem hiding this comment.
the parse_keyword still remains, the goal of introducing peek_keyword is that parse_keyword moves into the parse_unpivot_expr_table_factor function as an expect call instead
| redshift().verified_stmt("SELECT t.id, k, v FROM test_colors AS t, UNPIVOT t.count_by_color AS v AT k",); | ||
| redshift().verified_stmt("SELECT t.id, k, v FROM test_colors AS t, UNPIVOT t AS v AT k"); |
There was a problem hiding this comment.
can we change the dialect selection to use all_dialects_where(|d| d.supports...)?
|
Hi @iffyio , |
2 participants
Added support for unpivot in Redshift with expression and bracketsless:
SELECT t.id, k, v FROM test_colors as t, UNPIVOT t.count_by_color AS v AT k;