Skip to content

Conversation

@jimexist
Copy link
Member

No description provided.

@coveralls
Copy link

coveralls commented Nov 22, 2021

Pull Request Test Coverage Report for Build 1563508887

  • 144 of 150 (96.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.1%) to 89.793%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/parser.rs 31 37 83.78%
Files with Coverage Reduction New Missed Lines %
src/ast/mod.rs 1 75.74%
Totals Coverage Status
Change from base Build 1555940111: 0.1%
Covered Lines: 6158
Relevant Lines: 6858

💛 - Coveralls

@jimexist jimexist force-pushed the cube-rollup-grouping-sets branch from 2684527 to 6670d50 Compare November 22, 2021 01:56
@jimexist jimexist marked this pull request as ready for review November 22, 2021 01:58
@jimexist
Copy link
Member Author

need to rebase onto #367

src/parser.rs Outdated
/// parse a group by expr. a group by expr can be one of group sets, roll up, cube, or simple
/// expr.
fn parse_group_by_expr(&mut self) -> Result<Expr, ParserError> {
if self.parse_keywords(&[Keyword::GROUPING, Keyword::SETS]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I double checked at MySql also supports GROUPING, though it seems it supports WITH ROLLUP rather than ROLLUP, https://dev.mysql.com/doc/refman/8.0/en/group-by-modifiers.html

but seemingly not CUBE https://dev.mysql.com/doc/search/?d=201&p=1&q=cube

I think we should make parsing this part of the syntax conditional on the the postgresql dialect

Copy link
Member Author

Choose a reason for hiding this comment

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

done now

@jimexist jimexist force-pushed the cube-rollup-grouping-sets branch from 6670d50 to ddf96c7 Compare December 9, 2021 07:19
@jimexist jimexist force-pushed the cube-rollup-grouping-sets branch from ddf96c7 to b415c8f Compare December 10, 2021 13:01
@jimexist jimexist requested review from alamb and houqp December 10, 2021 13:02
@jimexist jimexist changed the title parse grouping sets, rollup, and cube parse grouping sets, rollup, and cube for postgresql Dec 10, 2021
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @jimexist !

@alamb alamb merged commit 40d67aa into apache:main Dec 10, 2021
@jimexist jimexist deleted the cube-rollup-grouping-sets branch December 11, 2021 11:38
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.

4 participants