Skip to content

Added support for unpivot in Redshift with expression and bracketsless#2375

Open
kfirSatori wants to merge 2 commits into
apache:mainfrom
SatoriCyber:upivot_expression_support
Open

Added support for unpivot in Redshift with expression and bracketsless#2375
kfirSatori wants to merge 2 commits into
apache:mainfrom
SatoriCyber:upivot_expression_support

Conversation

@kfirSatori

Copy link
Copy Markdown

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;

@kfirSatori

Copy link
Copy Markdown
Author

@iffyio Can you help me with the PR, it's a while here.

Comment thread src/ast/query.rs
/// Syntax:
/// ```sql
/// UNPIVOT expression AS value_alias [AT attribute_alias]
/// ```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a link to the docs describing the syntax?

Comment thread src/dialect/mod.rs
/// ```sql
/// UNPIVOT expression AS value_alias [AT attribute_alias]
/// ```
fn supports_unpivot_expr_in_from(&self) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn supports_unpivot_expr_in_from(&self) -> bool {
fn supports_unpivot_expr(&self) -> bool {

Comment on lines +547 to +561
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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/parser/mod.rs
with_ordinality,
})
} else if self.dialect.supports_unpivot_expr_in_from()
&& self.parse_keyword(Keyword::UNPIVOT)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread src/dialect/redshift.rs
true
}

fn supports_unpivot_expr_in_from(&self) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a link to the redshift docs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants