Skip to content

Many rows in totals#14500

Merged
KochetovNicolai merged 3 commits intomasterfrom
many-rows-in-totals
Sep 8, 2020
Merged

Many rows in totals#14500
KochetovNicolai merged 3 commits intomasterfrom
many-rows-in-totals

Conversation

@KochetovNicolai
Copy link
Copy Markdown
Member

@KochetovNicolai KochetovNicolai commented Sep 4, 2020

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

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Fix #12460

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 4, 2020
@tavplubix tavplubix self-assigned this Sep 4, 2020
alexey-milovidov
alexey-milovidov approved these changes Sep 4, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

Ok.

The result can be also surprising after array join with empty array.

@tavplubix
Copy link
Copy Markdown
Member

tavplubix commented Sep 7, 2020

Now most formats cut totals block to one row, but Pretty do not and it may print multiple rows. Maybe always print only the first row of totals block?

@KochetovNicolai
Copy link
Copy Markdown
Member Author

KochetovNicolai commented Sep 7, 2020

Now most formats cut totals block to one row, but Pretty do not and it may print multiple rows. Maybe always print only the first row of totals block?

Yes, I had the same idea. However, it cause backward incompatibility.
Probably, it is not significant and we can easily brake it. @alexey-milovidov what do you think?

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Sep 7, 2020

What incompatibility is possible?

@KochetovNicolai
Copy link
Copy Markdown
Member Author

KochetovNicolai commented Sep 7, 2020

Output result for queries like in test will be different in Pretty format (single totals rows instead of many).
I suppose, it was working that way for a log time .

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Sep 7, 2020

I's Ok to break.

@KochetovNicolai KochetovNicolai merged commit d771ce5 into master Sep 8, 2020
@KochetovNicolai KochetovNicolai deleted the many-rows-in-totals branch September 8, 2020 07:40
@Akazz Akazz mentioned this pull request Sep 11, 2020
Merged
@alexey-milovidov alexey-milovidov mentioned this pull request Nov 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tavplubix tavplubix tavplubix approved these changes

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

Assignees

@tavplubix tavplubix

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

clickhouse-client: Got 2 in totals chunk, expected 1

4 participants

@KochetovNicolai @tavplubix @alexey-milovidov @robot-clickhouse