-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLICKHOUSE-3893 - Add modificator rollup. #2948
Conversation
This reverts commit df017f9.
|
||
SELECT a, b, sum(s), count() from test.rollup GROUP BY ROLLUP(a, b) ORDER BY a, b; | ||
|
||
SELECT a, b, sum(s), count() from test.rollup GROUP BY ROLLUP(a, b) WITH TOTALS ORDER BY a, b; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not GROUP BY a, b WITH ROLLUP
? This benefit is friendly to people from mysql : )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will add both variants (MySQL style and standard SQL style).
Submodules are updated accidentially. |
dbms/src/Interpreters/Aggregator.h
Outdated
@@ -844,7 +844,7 @@ struct AggregatedDataVariants : private boost::noncopyable | |||
throw Exception("Unknown aggregated data variant.", ErrorCodes::UNKNOWN_AGGREGATED_DATA_VARIANT); | |||
} | |||
} | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
} | ||
|
||
/// WITH TOTALS | ||
/// WITH TOTALS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
const Settings & settings = context.getSettingsRef(); | ||
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
@@ -846,9 +861,10 @@ void InterpreterSelectQuery::executeAggregation(Pipeline & pipeline, const Expre | |||
if (descr.arguments.empty()) | |||
for (const auto & name : descr.argument_names) | |||
descr.arguments.push_back(header.getPositionByName(name)); | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
@@ -546,9 +555,12 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt | |||
else | |||
{ | |||
need_second_distinct_pass = query.distinct && pipeline.hasMoreThanOneStream(); | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
settings.max_bytes_before_external_group_by, settings.empty_result_for_aggregation_by_empty_set, | ||
context.getTemporaryPath()); | ||
|
||
pipeline.firstStream() = std::make_shared<RollupBlockInputStream>(pipeline.firstStream(), params); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indent.
|
||
Block rollup_block = block; | ||
|
||
for (int i = static_cast<int>(params.keys_size) - 1; i >= 0; --i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ssize_t
current.column = current.column->cloneEmpty()->cloneResized(rollup_block.rows()); | ||
|
||
BlocksList rollup_blocks = { rollup_block }; | ||
rollup_block = aggregator.mergeBlocks(rollup_blocks, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
{ | ||
MutableColumnPtr column = block.getByPosition(i).column->assumeMutable(); | ||
for (const auto & current_block : blocks) | ||
column->insertRangeFrom(*current_block.getByPosition(i).column.get(), 0, current_block.rows()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not necessarily to concatenate a block with rolled-up blocks.
You can return them one by one on next iterations of readImpl
(without reading next block from the source stream).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Submodules have leaked.
This reverts commit cdb2c8a.
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en