From 8c9fe80d2d1cdd3cc86219b468cc542b1b9f2f7e Mon Sep 17 00:00:00 2001 From: CurtizJ Date: Mon, 17 Sep 2018 21:01:04 +0300 Subject: [PATCH 01/10] add modificator cube --- dbms/src/DataStreams/CubeBlockInputStream.cpp | 61 +++++++++++++++++++ dbms/src/DataStreams/CubeBlockInputStream.h | 41 +++++++++++++ .../Interpreters/InterpreterSelectQuery.cpp | 20 ++++-- .../src/Interpreters/InterpreterSelectQuery.h | 2 +- dbms/src/Parsers/ASTSelectQuery.h | 1 + dbms/src/Parsers/ParserSelectQuery.cpp | 14 +++-- 6 files changed, 126 insertions(+), 13 deletions(-) create mode 100644 dbms/src/DataStreams/CubeBlockInputStream.cpp create mode 100644 dbms/src/DataStreams/CubeBlockInputStream.h diff --git a/dbms/src/DataStreams/CubeBlockInputStream.cpp b/dbms/src/DataStreams/CubeBlockInputStream.cpp new file mode 100644 index 00000000000..b178e36e849 --- /dev/null +++ b/dbms/src/DataStreams/CubeBlockInputStream.cpp @@ -0,0 +1,61 @@ +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +CubeBlockInputStream::CubeBlockInputStream( + const BlockInputStreamPtr & input_, const Aggregator::Params & params_) : aggregator(params_), + keys(params_.keys) +{ + children.push_back(input_); + Aggregator::CancellationHook hook = [this]() { return this->isCancelled(); }; + aggregator.setCancellationHook(hook); +} + + +Block CubeBlockInputStream::getHeader() const +{ + Block res = children.at(0)->getHeader(); + finalizeBlock(res); + return res; +} + + +Block CubeBlockInputStream::readImpl() +{ + /** After reading a block from input stream, + * we will subsequently roll it up on next iterations of 'readImpl' + * by zeroing out every column one-by-one and re-merging a block. + */ + + if (mask) + { + --mask; + Block cube_block = source_block; + for (size_t i = 0; i < keys.size(); ++i) + { + if (!((mask >> i) & 1)) + { + auto & current = cube_block.getByPosition(keys[keys.size() - i - 1]); + current.column = current.column->cloneEmpty()->cloneResized(cube_block.rows()); + } + } + + BlocksList cube_blocks = { cube_block }; + Block finalized = aggregator.mergeBlocks(cube_blocks, true); + return finalized; + } + + source_block = children[0]->read(); + Block finalized = source_block; + finalizeBlock(finalized); + mask = (1 << keys.size()) - 1; + + return finalized; +} +} diff --git a/dbms/src/DataStreams/CubeBlockInputStream.h b/dbms/src/DataStreams/CubeBlockInputStream.h new file mode 100644 index 00000000000..5f58fc5dc9a --- /dev/null +++ b/dbms/src/DataStreams/CubeBlockInputStream.h @@ -0,0 +1,41 @@ +#pragma once + +#include +#include +#include +#include + + +namespace DB +{ + +class ExpressionActions; + + +/** Takes blocks after grouping, with non-finalized aggregate functions. + * Calculates subtotals and grand totals values for a set of columns. + */ +class CubeBlockInputStream : public IProfilingBlockInputStream +{ +private: + using ExpressionActionsPtr = std::shared_ptr; + using AggregateColumns = std::vector; +public: + CubeBlockInputStream( + const BlockInputStreamPtr & input_, const Aggregator::Params & params_); + + String getName() const override { return "Cube"; } + + Block getHeader() const override; + +protected: + Block readImpl() override; + +private: + Aggregator aggregator; + ColumnNumbers keys; + size_t mask = 0; + Block source_block; +}; + +} diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp index 6eecbca144f..00a79bf8fbf 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp +++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -499,7 +500,7 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt bool aggregate_final = expressions.need_aggregate && to_stage > QueryProcessingStage::WithMergeableState && - !query.group_by_with_totals && !query.group_by_with_rollup; + !query.group_by_with_totals && !query.group_by_with_rollup && !query.group_by_with_cube; if (expressions.first_stage) { @@ -559,8 +560,10 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt if (query.group_by_with_totals) executeTotalsAndHaving(pipeline, expressions.has_having, expressions.before_having, aggregate_overflow_row, !query.group_by_with_rollup); - if (query.group_by_with_rollup) - executeRollup(pipeline); + if (query.group_by_with_rollup) + executeRollupOrCube(pipeline, true); + else if(query.group_by_with_cube) + executeRollupOrCube(pipeline, false); } else if (expressions.has_having) executeHaving(pipeline, expressions.before_having); @@ -578,7 +581,9 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt executeTotalsAndHaving(pipeline, false, nullptr, aggregate_overflow_row, !query.group_by_with_rollup); if (query.group_by_with_rollup && !aggregate_final) - executeRollup(pipeline); + executeRollupOrCube(pipeline, true); + else if (query.group_by_with_cube && !aggregate_final) + executeRollupOrCube(pipeline, false); } if (expressions.has_order_by) @@ -1087,7 +1092,7 @@ void InterpreterSelectQuery::executeTotalsAndHaving(Pipeline & pipeline, bool ha has_having ? query.having_expression->getColumnName() : "", settings.totals_mode, settings.totals_auto_threshold, final); } -void InterpreterSelectQuery::executeRollup(Pipeline & pipeline) +void InterpreterSelectQuery::executeRollupOrCube(Pipeline & pipeline, bool is_rollup) { executeUnion(pipeline); @@ -1111,7 +1116,10 @@ void InterpreterSelectQuery::executeRollup(Pipeline & pipeline) settings.max_bytes_before_external_group_by, settings.empty_result_for_aggregation_by_empty_set, context.getTemporaryPath()); - pipeline.firstStream() = std::make_shared(pipeline.firstStream(), params); + if (is_rollup) + pipeline.firstStream() = std::make_shared(pipeline.firstStream(), params); + else + pipeline.firstStream() = std::make_shared(pipeline.firstStream(), params); } diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.h b/dbms/src/Interpreters/InterpreterSelectQuery.h index 9c534c2846a..06042417400 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.h +++ b/dbms/src/Interpreters/InterpreterSelectQuery.h @@ -190,7 +190,7 @@ private: void executeDistinct(Pipeline & pipeline, bool before_order, Names columns); void executeExtremes(Pipeline & pipeline); void executeSubqueriesInSetsAndJoins(Pipeline & pipeline, std::unordered_map & subqueries_for_sets); - void executeRollup(Pipeline & pipeline); + void executeRollupOrCube(Pipeline & pipeline, bool is_rollup); /** If there is a SETTINGS section in the SELECT query, then apply settings from it. * diff --git a/dbms/src/Parsers/ASTSelectQuery.h b/dbms/src/Parsers/ASTSelectQuery.h index 7422c70212c..0ffdb44395e 100644 --- a/dbms/src/Parsers/ASTSelectQuery.h +++ b/dbms/src/Parsers/ASTSelectQuery.h @@ -29,6 +29,7 @@ public: ASTPtr group_expression_list; bool group_by_with_totals = false; bool group_by_with_rollup = false; + bool group_by_with_cube = false; ASTPtr having_expression; ASTPtr order_expression_list; ASTPtr limit_by_value; diff --git a/dbms/src/Parsers/ParserSelectQuery.cpp b/dbms/src/Parsers/ParserSelectQuery.cpp index 480cb32b8bd..293c75af5d7 100644 --- a/dbms/src/Parsers/ParserSelectQuery.cpp +++ b/dbms/src/Parsers/ParserSelectQuery.cpp @@ -40,6 +40,7 @@ bool ParserSelectQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) ParserKeyword s_settings("SETTINGS"); ParserKeyword s_by("BY"); ParserKeyword s_rollup("ROLLUP"); + ParserKeyword s_cube("CUBE"); ParserKeyword s_top("TOP"); ParserKeyword s_offset("OFFSET"); @@ -116,20 +117,21 @@ bool ParserSelectQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) if (s_group_by.ignore(pos, expected)) { if (s_rollup.ignore(pos, expected)) - { select_query->group_by_with_rollup = true; - if (!open_bracket.ignore(pos, expected)) - return false; - } + else if (s_cube.ignore(pos, expected)) + select_query->group_by_with_cube = true; + + if ((select_query->group_by_with_rollup || select_query->group_by_with_cube) && !open_bracket.ignore(pos, expected)) + return false; if (!exp_list.parse(pos, select_query->group_expression_list, expected)) return false; - if (select_query->group_by_with_rollup && !close_bracket.ignore(pos, expected)) + if ((select_query->group_by_with_rollup || select_query->group_by_with_cube) && !close_bracket.ignore(pos, expected)) return false; } - /// WITH ROLLUP + /// WITH ROLLUP or TOTALS if (s_with.ignore(pos, expected)) { if (s_rollup.ignore(pos, expected)) From 43951e4879e9b73d72641b1aa4259be221cf96eb Mon Sep 17 00:00:00 2001 From: CurtizJ Date: Mon, 17 Sep 2018 22:16:51 +0300 Subject: [PATCH 02/10] add test --- dbms/src/DataStreams/CubeBlockInputStream.cpp | 8 +++++++ dbms/src/DataStreams/CubeBlockInputStream.h | 2 +- .../Interpreters/InterpreterSelectQuery.cpp | 10 ++++++-- .../0_stateless/00718_with_cube.reference | 23 +++++++++++++++++++ .../queries/0_stateless/00718_with_cube.sql | 17 ++++++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 dbms/tests/queries/0_stateless/00718_with_cube.reference create mode 100644 dbms/tests/queries/0_stateless/00718_with_cube.sql diff --git a/dbms/src/DataStreams/CubeBlockInputStream.cpp b/dbms/src/DataStreams/CubeBlockInputStream.cpp index b178e36e849..ec9467b1f86 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.cpp +++ b/dbms/src/DataStreams/CubeBlockInputStream.cpp @@ -8,10 +8,18 @@ namespace DB { +namespace ErrorCodes +{ + extern const int TOO_MANY_COLUMNS; +} + CubeBlockInputStream::CubeBlockInputStream( const BlockInputStreamPtr & input_, const Aggregator::Params & params_) : aggregator(params_), keys(params_.keys) { + if (keys.size() > 30) + throw Exception("Too many columns for cube", ErrorCodes::TOO_MANY_COLUMNS); + children.push_back(input_); Aggregator::CancellationHook hook = [this]() { return this->isCancelled(); }; aggregator.setCancellationHook(hook); diff --git a/dbms/src/DataStreams/CubeBlockInputStream.h b/dbms/src/DataStreams/CubeBlockInputStream.h index 5f58fc5dc9a..53cca98f654 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.h +++ b/dbms/src/DataStreams/CubeBlockInputStream.h @@ -34,7 +34,7 @@ protected: private: Aggregator aggregator; ColumnNumbers keys; - size_t mask = 0; + UInt32 mask = 0; Block source_block; }; diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp index 00a79bf8fbf..56477ce38e3 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp +++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp @@ -558,7 +558,10 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt if (!aggregate_final) { if (query.group_by_with_totals) - executeTotalsAndHaving(pipeline, expressions.has_having, expressions.before_having, aggregate_overflow_row, !query.group_by_with_rollup); + { + bool final = !query.group_by_with_rollup && !query.group_by_with_cube; + executeTotalsAndHaving(pipeline, expressions.has_having, expressions.before_having, aggregate_overflow_row, final); + } if (query.group_by_with_rollup) executeRollupOrCube(pipeline, true); @@ -578,7 +581,10 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt need_second_distinct_pass = query.distinct && pipeline.hasMoreThanOneStream(); if (query.group_by_with_totals && !aggregate_final) - executeTotalsAndHaving(pipeline, false, nullptr, aggregate_overflow_row, !query.group_by_with_rollup); + { + bool final = !query.group_by_with_rollup && !query.group_by_with_cube; + executeTotalsAndHaving(pipeline, expressions.has_having, expressions.before_having, aggregate_overflow_row, final); + } if (query.group_by_with_rollup && !aggregate_final) executeRollupOrCube(pipeline, true); diff --git a/dbms/tests/queries/0_stateless/00718_with_cube.reference b/dbms/tests/queries/0_stateless/00718_with_cube.reference new file mode 100644 index 00000000000..ae0eaa122af --- /dev/null +++ b/dbms/tests/queries/0_stateless/00718_with_cube.reference @@ -0,0 +1,23 @@ + 0 120 8 + 1 40 4 + 2 80 4 +a 0 70 4 +a 1 25 2 +a 2 45 2 +b 0 50 4 +b 1 15 2 +b 2 35 2 + 0 120 8 + 1 40 4 + 2 80 4 +a 0 70 4 +a 1 25 2 +a 2 45 2 +b 0 50 4 +b 1 15 2 +b 2 35 2 + + 0 120 8 + 120 8 +a 70 4 +b 50 4 diff --git a/dbms/tests/queries/0_stateless/00718_with_cube.sql b/dbms/tests/queries/0_stateless/00718_with_cube.sql new file mode 100644 index 00000000000..7b19119478f --- /dev/null +++ b/dbms/tests/queries/0_stateless/00718_with_cube.sql @@ -0,0 +1,17 @@ +DROP TABLE IF EXISTS test.rollup; +CREATE TABLE test.rollup(a String, b Int32, s Int32) ENGINE = Memory; + +INSERT INTO test.rollup VALUES('a', 1, 10); +INSERT INTO test.rollup VALUES('a', 1, 15); +INSERT INTO test.rollup VALUES('a', 2, 20); +INSERT INTO test.rollup VALUES('a', 2, 25); +INSERT INTO test.rollup VALUES('b', 1, 10); +INSERT INTO test.rollup VALUES('b', 1, 5); +INSERT INTO test.rollup VALUES('b', 2, 20); +INSERT INTO test.rollup VALUES('b', 2, 15); + +SELECT a, b, sum(s), count() from test.rollup GROUP BY CUBE(a, b) ORDER BY a, b; + +SELECT a, b, sum(s), count() from test.rollup GROUP BY CUBE(a, b) WITH TOTALS ORDER BY a, b; + +SELECT a, sum(s), count() from test.rollup GROUP BY CUBE(a) ORDER BY a; \ No newline at end of file From 31bf960bfedba307b79e86f4527aabb8ee8bc037 Mon Sep 17 00:00:00 2001 From: CurtizJ Date: Wed, 19 Sep 2018 14:18:38 +0300 Subject: [PATCH 03/10] add new syntax --- dbms/src/DataStreams/CubeBlockInputStream.cpp | 4 ++-- dbms/src/DataStreams/CubeBlockInputStream.h | 2 +- dbms/src/Parsers/ASTSelectQuery.cpp | 3 +++ dbms/src/Parsers/ParserSelectQuery.cpp | 4 +++- ..._cube.reference => 00720_with_cube.reference} | 16 ++++++++++++++++ .../{00718_with_cube.sql => 00720_with_cube.sql} | 6 +++++- 6 files changed, 30 insertions(+), 5 deletions(-) rename dbms/tests/queries/0_stateless/{00718_with_cube.reference => 00720_with_cube.reference} (58%) rename dbms/tests/queries/0_stateless/{00718_with_cube.sql => 00720_with_cube.sql} (78%) diff --git a/dbms/src/DataStreams/CubeBlockInputStream.cpp b/dbms/src/DataStreams/CubeBlockInputStream.cpp index ec9467b1f86..ee9b3f66036 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.cpp +++ b/dbms/src/DataStreams/CubeBlockInputStream.cpp @@ -37,8 +37,8 @@ Block CubeBlockInputStream::getHeader() const Block CubeBlockInputStream::readImpl() { /** After reading a block from input stream, - * we will subsequently roll it up on next iterations of 'readImpl' - * by zeroing out every column one-by-one and re-merging a block. + * we will calculate all subsets of columns on next iterations of readImpl + * by zeroing columns at positions, where bits are zero in current bitmask. */ if (mask) diff --git a/dbms/src/DataStreams/CubeBlockInputStream.h b/dbms/src/DataStreams/CubeBlockInputStream.h index 53cca98f654..ca2de5367bb 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.h +++ b/dbms/src/DataStreams/CubeBlockInputStream.h @@ -13,7 +13,7 @@ class ExpressionActions; /** Takes blocks after grouping, with non-finalized aggregate functions. - * Calculates subtotals and grand totals values for a set of columns. + * Calculates all subsets of columns and aggreagetes over them. */ class CubeBlockInputStream : public IProfilingBlockInputStream { diff --git a/dbms/src/Parsers/ASTSelectQuery.cpp b/dbms/src/Parsers/ASTSelectQuery.cpp index 0e48ac585b7..acb1d49f0f0 100644 --- a/dbms/src/Parsers/ASTSelectQuery.cpp +++ b/dbms/src/Parsers/ASTSelectQuery.cpp @@ -106,6 +106,9 @@ void ASTSelectQuery::formatImpl(const FormatSettings & s, FormatState & state, F if (group_by_with_rollup) s.ostr << (s.hilite ? hilite_keyword : "") << s.nl_or_ws << indent_str << (s.one_line ? "" : " ") << "WITH ROLLUP" << (s.hilite ? hilite_none : ""); + if (group_by_with_cube) + s.ostr << (s.hilite ? hilite_keyword : "") << s.nl_or_ws << indent_str << (s.one_line ? "" : " ") << "WITH CUBE" << (s.hilite ? hilite_none : ""); + if (group_by_with_totals) s.ostr << (s.hilite ? hilite_keyword : "") << s.nl_or_ws << indent_str << (s.one_line ? "" : " ") << "WITH TOTALS" << (s.hilite ? hilite_none : ""); diff --git a/dbms/src/Parsers/ParserSelectQuery.cpp b/dbms/src/Parsers/ParserSelectQuery.cpp index 293c75af5d7..ffd9273dd8a 100644 --- a/dbms/src/Parsers/ParserSelectQuery.cpp +++ b/dbms/src/Parsers/ParserSelectQuery.cpp @@ -131,11 +131,13 @@ bool ParserSelectQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) return false; } - /// WITH ROLLUP or TOTALS + /// WITH ROLLUP, CUBE or TOTALS if (s_with.ignore(pos, expected)) { if (s_rollup.ignore(pos, expected)) select_query->group_by_with_rollup = true; + else if (s_cube.ignore(pos, expected)) + select_query->group_by_with_cube = true; else if (s_totals.ignore(pos, expected)) select_query->group_by_with_totals = true; else diff --git a/dbms/tests/queries/0_stateless/00718_with_cube.reference b/dbms/tests/queries/0_stateless/00720_with_cube.reference similarity index 58% rename from dbms/tests/queries/0_stateless/00718_with_cube.reference rename to dbms/tests/queries/0_stateless/00720_with_cube.reference index ae0eaa122af..4a06b9d367c 100644 --- a/dbms/tests/queries/0_stateless/00718_with_cube.reference +++ b/dbms/tests/queries/0_stateless/00720_with_cube.reference @@ -21,3 +21,19 @@ b 2 35 2 120 8 a 70 4 b 50 4 + 0 120 8 +a 0 70 4 +a 1 25 2 +a 2 45 2 +b 0 50 4 +b 1 15 2 +b 2 35 2 + 0 120 8 +a 0 70 4 +a 1 25 2 +a 2 45 2 +b 0 50 4 +b 1 15 2 +b 2 35 2 + + 0 120 8 diff --git a/dbms/tests/queries/0_stateless/00718_with_cube.sql b/dbms/tests/queries/0_stateless/00720_with_cube.sql similarity index 78% rename from dbms/tests/queries/0_stateless/00718_with_cube.sql rename to dbms/tests/queries/0_stateless/00720_with_cube.sql index 7b19119478f..e7e63e7e6a3 100644 --- a/dbms/tests/queries/0_stateless/00718_with_cube.sql +++ b/dbms/tests/queries/0_stateless/00720_with_cube.sql @@ -14,4 +14,8 @@ SELECT a, b, sum(s), count() from test.rollup GROUP BY CUBE(a, b) ORDER BY a, b; SELECT a, b, sum(s), count() from test.rollup GROUP BY CUBE(a, b) WITH TOTALS ORDER BY a, b; -SELECT a, sum(s), count() from test.rollup GROUP BY CUBE(a) ORDER BY a; \ No newline at end of file +SELECT a, sum(s), count() from test.rollup GROUP BY CUBE(a) ORDER BY a; + +SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH ROLLUP ORDER BY a; + +SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH ROLLUP WITH TOTALS ORDER BY a; \ No newline at end of file From f9ff58641545037a7937e139d0479ba3ce5c6f69 Mon Sep 17 00:00:00 2001 From: CurtizJ Date: Thu, 20 Sep 2018 13:44:13 +0300 Subject: [PATCH 04/10] optimize --- dbms/src/DataStreams/CubeBlockInputStream.cpp | 6 ++++-- dbms/src/DataStreams/CubeBlockInputStream.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dbms/src/DataStreams/CubeBlockInputStream.cpp b/dbms/src/DataStreams/CubeBlockInputStream.cpp index ee9b3f66036..2253524c638 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.cpp +++ b/dbms/src/DataStreams/CubeBlockInputStream.cpp @@ -49,8 +49,9 @@ Block CubeBlockInputStream::readImpl() { if (!((mask >> i) & 1)) { - auto & current = cube_block.getByPosition(keys[keys.size() - i - 1]); - current.column = current.column->cloneEmpty()->cloneResized(cube_block.rows()); + size_t pos = keys.size() - i - 1; + auto & current = cube_block.getByPosition(keys[pos]); + current.column = empty_block.getByPosition(pos).column->cloneResized(cube_block.rows()); } } @@ -60,6 +61,7 @@ Block CubeBlockInputStream::readImpl() } source_block = children[0]->read(); + empty_block = source_block.cloneEmpty(); Block finalized = source_block; finalizeBlock(finalized); mask = (1 << keys.size()) - 1; diff --git a/dbms/src/DataStreams/CubeBlockInputStream.h b/dbms/src/DataStreams/CubeBlockInputStream.h index ca2de5367bb..d399e44b6d5 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.h +++ b/dbms/src/DataStreams/CubeBlockInputStream.h @@ -36,6 +36,7 @@ private: ColumnNumbers keys; UInt32 mask = 0; Block source_block; + Block empty_block; }; } From 8644853558e983f13c7f5df045cb37d1c7e58fa4 Mon Sep 17 00:00:00 2001 From: CurtizJ Date: Thu, 20 Sep 2018 18:46:37 +0300 Subject: [PATCH 05/10] optimize --- dbms/src/DataStreams/CubeBlockInputStream.cpp | 19 +++++++++++++------ dbms/src/DataStreams/CubeBlockInputStream.h | 2 +- .../Interpreters/InterpreterSelectQuery.cpp | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/dbms/src/DataStreams/CubeBlockInputStream.cpp b/dbms/src/DataStreams/CubeBlockInputStream.cpp index 2253524c638..ae9fd28e681 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.cpp +++ b/dbms/src/DataStreams/CubeBlockInputStream.cpp @@ -40,18 +40,17 @@ Block CubeBlockInputStream::readImpl() * we will calculate all subsets of columns on next iterations of readImpl * by zeroing columns at positions, where bits are zero in current bitmask. */ - - if (mask) + if (mask) { --mask; Block cube_block = source_block; - for (size_t i = 0; i < keys.size(); ++i) + for (size_t i = 0; i < keys.size(); ++i) { - if (!((mask >> i) & 1)) + if (!((mask >> i) & 1)) { size_t pos = keys.size() - i - 1; auto & current = cube_block.getByPosition(keys[pos]); - current.column = empty_block.getByPosition(pos).column->cloneResized(cube_block.rows()); + current.column = zero_block.getByPosition(pos).column; } } @@ -61,7 +60,15 @@ Block CubeBlockInputStream::readImpl() } source_block = children[0]->read(); - empty_block = source_block.cloneEmpty(); + if (!source_block) + return source_block; + + zero_block = source_block.cloneEmpty(); + for (auto key : keys) + { + auto & current = zero_block.getByPosition(key); + current.column = current.column->cloneResized(source_block.rows()); + } Block finalized = source_block; finalizeBlock(finalized); mask = (1 << keys.size()) - 1; diff --git a/dbms/src/DataStreams/CubeBlockInputStream.h b/dbms/src/DataStreams/CubeBlockInputStream.h index d399e44b6d5..46f1a5bb432 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.h +++ b/dbms/src/DataStreams/CubeBlockInputStream.h @@ -36,7 +36,7 @@ private: ColumnNumbers keys; UInt32 mask = 0; Block source_block; - Block empty_block; + Block zero_block; }; } diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp index 56477ce38e3..72da70b4b74 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp +++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp @@ -1124,7 +1124,7 @@ void InterpreterSelectQuery::executeRollupOrCube(Pipeline & pipeline, bool is_ro if (is_rollup) pipeline.firstStream() = std::make_shared(pipeline.firstStream(), params); - else + else pipeline.firstStream() = std::make_shared(pipeline.firstStream(), params); } From 5f2db689dc64fe5d0f95f3e5b86ee2009b820f86 Mon Sep 17 00:00:00 2001 From: CurtizJ Date: Thu, 20 Sep 2018 19:32:07 +0300 Subject: [PATCH 06/10] fix tests --- dbms/tests/queries/0_stateless/00720_with_cube.reference | 7 ++++--- dbms/tests/queries/0_stateless/00720_with_cube.sql | 6 ++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dbms/tests/queries/0_stateless/00720_with_cube.reference b/dbms/tests/queries/0_stateless/00720_with_cube.reference index 4a06b9d367c..a0b951978f9 100644 --- a/dbms/tests/queries/0_stateless/00720_with_cube.reference +++ b/dbms/tests/queries/0_stateless/00720_with_cube.reference @@ -18,17 +18,18 @@ b 1 15 2 b 2 35 2 0 120 8 - 120 8 -a 70 4 -b 50 4 + 1 40 4 0 120 8 + 2 80 4 a 0 70 4 a 1 25 2 a 2 45 2 b 0 50 4 b 1 15 2 b 2 35 2 + 1 40 4 0 120 8 + 2 80 4 a 0 70 4 a 1 25 2 a 2 45 2 diff --git a/dbms/tests/queries/0_stateless/00720_with_cube.sql b/dbms/tests/queries/0_stateless/00720_with_cube.sql index e7e63e7e6a3..0a5ea2a5b61 100644 --- a/dbms/tests/queries/0_stateless/00720_with_cube.sql +++ b/dbms/tests/queries/0_stateless/00720_with_cube.sql @@ -14,8 +14,6 @@ SELECT a, b, sum(s), count() from test.rollup GROUP BY CUBE(a, b) ORDER BY a, b; SELECT a, b, sum(s), count() from test.rollup GROUP BY CUBE(a, b) WITH TOTALS ORDER BY a, b; -SELECT a, sum(s), count() from test.rollup GROUP BY CUBE(a) ORDER BY a; +SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH CUBE ORDER BY a; -SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH ROLLUP ORDER BY a; - -SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH ROLLUP WITH TOTALS ORDER BY a; \ No newline at end of file +SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH CUBE WITH TOTALS ORDER BY a; \ No newline at end of file From dd1b130048ba8ab60dc2833495767eefa84a8f98 Mon Sep 17 00:00:00 2001 From: Anton Popov Date: Thu, 20 Sep 2018 20:32:57 +0300 Subject: [PATCH 07/10] Update CubeBlockInputStream.cpp --- dbms/src/DataStreams/CubeBlockInputStream.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/DataStreams/CubeBlockInputStream.cpp b/dbms/src/DataStreams/CubeBlockInputStream.cpp index ae9fd28e681..ecada4534d1 100644 --- a/dbms/src/DataStreams/CubeBlockInputStream.cpp +++ b/dbms/src/DataStreams/CubeBlockInputStream.cpp @@ -19,7 +19,7 @@ CubeBlockInputStream::CubeBlockInputStream( { if (keys.size() > 30) throw Exception("Too many columns for cube", ErrorCodes::TOO_MANY_COLUMNS); - + children.push_back(input_); Aggregator::CancellationHook hook = [this]() { return this->isCancelled(); }; aggregator.setCancellationHook(hook); From 7a9500a3f13f0e63e2e6d0c775a5890497266f11 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Thu, 20 Sep 2018 20:33:47 +0300 Subject: [PATCH 08/10] Update InterpreterSelectQuery.cpp --- dbms/src/Interpreters/InterpreterSelectQuery.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp index 72da70b4b74..98629451c82 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp +++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp @@ -565,7 +565,7 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt if (query.group_by_with_rollup) executeRollupOrCube(pipeline, true); - else if(query.group_by_with_cube) + else if (query.group_by_with_cube) executeRollupOrCube(pipeline, false); } else if (expressions.has_having) From f558d5b2df0b54c3c726bb593c31198b3e5e8c3a Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Thu, 20 Sep 2018 20:35:54 +0300 Subject: [PATCH 09/10] Update 00720_with_cube.sql --- dbms/tests/queries/0_stateless/00720_with_cube.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/tests/queries/0_stateless/00720_with_cube.sql b/dbms/tests/queries/0_stateless/00720_with_cube.sql index 0a5ea2a5b61..032c83f17b5 100644 --- a/dbms/tests/queries/0_stateless/00720_with_cube.sql +++ b/dbms/tests/queries/0_stateless/00720_with_cube.sql @@ -16,4 +16,4 @@ SELECT a, b, sum(s), count() from test.rollup GROUP BY CUBE(a, b) WITH TOTALS OR SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH CUBE ORDER BY a; -SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH CUBE WITH TOTALS ORDER BY a; \ No newline at end of file +SELECT a, b, sum(s), count() from test.rollup GROUP BY a, b WITH CUBE WITH TOTALS ORDER BY a; From b89feb1572efe751ba14c742ea6a3978e2a6bd5c Mon Sep 17 00:00:00 2001 From: CurtizJ Date: Thu, 20 Sep 2018 20:51:42 +0300 Subject: [PATCH 10/10] replace bool by enum --- dbms/src/Interpreters/InterpreterSelectQuery.cpp | 12 ++++++------ dbms/src/Interpreters/InterpreterSelectQuery.h | 9 ++++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp index 72da70b4b74..a9b928c1cb3 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp +++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp @@ -564,9 +564,9 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt } if (query.group_by_with_rollup) - executeRollupOrCube(pipeline, true); + executeRollupOrCube(pipeline, Modificator::ROLLUP); else if(query.group_by_with_cube) - executeRollupOrCube(pipeline, false); + executeRollupOrCube(pipeline, Modificator::CUBE); } else if (expressions.has_having) executeHaving(pipeline, expressions.before_having); @@ -587,9 +587,9 @@ void InterpreterSelectQuery::executeImpl(Pipeline & pipeline, const BlockInputSt } if (query.group_by_with_rollup && !aggregate_final) - executeRollupOrCube(pipeline, true); + executeRollupOrCube(pipeline, Modificator::ROLLUP); else if (query.group_by_with_cube && !aggregate_final) - executeRollupOrCube(pipeline, false); + executeRollupOrCube(pipeline, Modificator::CUBE); } if (expressions.has_order_by) @@ -1098,7 +1098,7 @@ void InterpreterSelectQuery::executeTotalsAndHaving(Pipeline & pipeline, bool ha has_having ? query.having_expression->getColumnName() : "", settings.totals_mode, settings.totals_auto_threshold, final); } -void InterpreterSelectQuery::executeRollupOrCube(Pipeline & pipeline, bool is_rollup) +void InterpreterSelectQuery::executeRollupOrCube(Pipeline & pipeline, Modificator modificator) { executeUnion(pipeline); @@ -1122,7 +1122,7 @@ void InterpreterSelectQuery::executeRollupOrCube(Pipeline & pipeline, bool is_ro settings.max_bytes_before_external_group_by, settings.empty_result_for_aggregation_by_empty_set, context.getTemporaryPath()); - if (is_rollup) + if (modificator == Modificator::ROLLUP) pipeline.firstStream() = std::make_shared(pipeline.firstStream(), params); else pipeline.firstStream() = std::make_shared(pipeline.firstStream(), params); diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.h b/dbms/src/Interpreters/InterpreterSelectQuery.h index 06042417400..8d433ab1754 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.h +++ b/dbms/src/Interpreters/InterpreterSelectQuery.h @@ -190,7 +190,14 @@ private: void executeDistinct(Pipeline & pipeline, bool before_order, Names columns); void executeExtremes(Pipeline & pipeline); void executeSubqueriesInSetsAndJoins(Pipeline & pipeline, std::unordered_map & subqueries_for_sets); - void executeRollupOrCube(Pipeline & pipeline, bool is_rollup); + + enum class Modificator + { + ROLLUP = 0, + CUBE = 1 + }; + + void executeRollupOrCube(Pipeline & pipeline, Modificator modificator); /** If there is a SETTINGS section in the SELECT query, then apply settings from it. *