Skip to content
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

Throw exceptions if WITH TOTALS/ROLLUP/CUBE are specified without agg… #6976

Merged
merged 3 commits into from Sep 20, 2019

Conversation

sfod
Copy link
Contributor

@sfod sfod commented Sep 18, 2019

…regate functions

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

For changelog. Remove if this is non-significant change.

Category (leave one):

  • Bug Fix

Short description (up to few sentences):
WITH TOTALS make sense only when you apply aggregation.
...

Detailed description (optional):

...

@sfod
Copy link
Contributor Author

sfod commented Sep 19, 2019

I've missed two tests using WITH TOTALS without aggregation: 00937_template_output_format and 00378_json_quote_64bit_integers.
I could fix these two tests by removing WITH TOTALS or adding aggregate functions if the fix itself is fine.

Copy link
Member

@CurtizJ CurtizJ left a comment

Choose a reason for hiding this comment

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

Your changes are ok. It's better to add aggregation to failing tests, because they check formats for correct representation of totals block.


# Must throw an exception
EXCEPTION_TEXT="WITH TOTALS, ROLLUP or CUBE are not supported without aggregation"
$CLICKHOUSE_CLIENT --query="SELECT 1 AS id, 'hello' AS s WITH TOTALS" 2>&1 \
Copy link
Member

@CurtizJ CurtizJ Sep 20, 2019

Choose a reason for hiding this comment

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

It's more convenient to write simple sql test and add special comment after query
-- { serverError 49 }, which means that client would expect exception with code 49.

@alexey-milovidov alexey-milovidov merged commit 3f500aa into ClickHouse:master Sep 20, 2019
@vitlibar vitlibar added the pr-bugfix Pull request with bugfix, not backported by default label Dec 26, 2019
vitlibar pushed a commit that referenced this pull request Dec 26, 2019
Throw exceptions if WITH TOTALS/ROLLUP/CUBE are specified without agg…

(cherry picked from commit 3f500aa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants