feat(cubestore): Aggregating index #4379
Conversation
github-actions
bot
added
the
pr:community
rpaik
added
Roadmap: 2022 Q2
rpaik
added
backend:pre-aggregations
| tokio::fs::File::create(path_2.clone()).await.unwrap(), | ||
| )); | ||
|
|
||
| file.write_all("platform,age,gender,cnt,max_id\n".as_bytes()) |
There was a problem hiding this comment.
Creating test tables by hand is getting tedious. Can we please extract it into a function and invoke it like
let paths = create_test_files([
r#"
"iOS", 20, "M", 10, 100,
"android", 20, "M", 2, 10,
<etc>
"#
])
There was a problem hiding this comment.
Yes, that's right, I'll do this refactoring next time I need to create such table for tests.
| loop { | ||
| let is_aggregate = self.parse_custom_token("aggregate"); | ||
|
|
||
| if self.parser.parse_keyword(Keyword::INDEX) { |
There was a problem hiding this comment.
This is a bit confusing. How about:
if self.parse_custom_token("aggregate").is_ok() {
self.parser.parse_keyword(Keyword::INDEX)?
indexes.push(self.parse_with_index(name.clone(), true)?;
} else if self.parser.parse_keyword(Keyword::INDEX).is_ok() {
indexes.push(self.parse_with_index(name.clone(), false)?;
} else {
break;
}
There was a problem hiding this comment.
Yes, I probably will do that
There was a problem hiding this comment.
The comment say's "done at 286edbd", but I still see the old version. Perhaps you forgot tp push, or I'm seeing a stale version from some weird reason?
| ) -> Result<SQLStatement, ParserError> { | ||
| let index_name = self.parser.parse_object_name()?; | ||
| self.parser.expect_token(&Token::LParen)?; | ||
| let columns = self | ||
| .parser | ||
| .parse_comma_separated(Parser::parse_order_by_expr)?; | ||
| self.parser.expect_token(&Token::RParen)?; | ||
| //TODO I use unique flag for aggregate index for reusing CreateIndex struct. When adding another type of index, we will need to parse it into a custom structure |
There was a problem hiding this comment.
Nooooooo ;) Can we please add a separate flag and not re-use something with the wrong name/semantics?
There was a problem hiding this comment.
We can't add a separate flag, because it is structure from sql-parser.rs, but we can write our own structure and write our own parser flow for indexes. Maybe you're right and I should do it.
There was a problem hiding this comment.
Not a big fan of accumulating differences in private forks of OS projects, but adding a flag to a struct sounds like a minimal change. https://github.com/cube-js/sqlparser-rs
Yep, it's a first sub task. I'll merge next steps into this branch, so at the end it will be full implementation of aggregate index |
| loop { | ||
| let is_aggregate = self.parse_custom_token("aggregate"); | ||
|
|
||
| if self.parser.parse_keyword(Keyword::INDEX) { |
There was a problem hiding this comment.
The comment say's "done at 286edbd", but I still see the old version. Perhaps you forgot tp push, or I'm seeing a stale version from some weird reason?
| ) -> Result<SQLStatement, ParserError> { | ||
| let index_name = self.parser.parse_object_name()?; | ||
| self.parser.expect_token(&Token::LParen)?; | ||
| let columns = self | ||
| .parser | ||
| .parse_comma_separated(Parser::parse_order_by_expr)?; | ||
| self.parser.expect_token(&Token::RParen)?; | ||
| //TODO I use unique flag for aggregate index for reusing CreateIndex struct. When adding another type of index, we will need to parse it into a custom structure |
There was a problem hiding this comment.
Not a big fan of accumulating differences in private forks of OS projects, but adding a flag to a struct sounds like a minimal change. https://github.com/cube-js/sqlparser-rs
…ures/aggregating_index
…ures/aggregating_index
Check List
Description of Changes Made (if issue reference is not provided)
Implementation of aggregating indexes