Skip to content

Add group_by_use_nulls setting#38642

Merged
novikd merged 18 commits intomasterfrom
group-by-use-nulls
Jul 20, 2022
Merged

Add group_by_use_nulls setting#38642
novikd merged 18 commits intomasterfrom
group-by-use-nulls

Conversation

@novikd
Copy link
Copy Markdown
Member

@novikd novikd commented Jun 30, 2022

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Add group_by_use_nulls setting to make aggregation key columns nullable in the case of ROLLUP, CUBE and GROUPING SETS. Closes #37359

@tavplubix
Copy link
Copy Markdown
Member

Consider adding it here:

def get_options(i, backward_compatibility_check):

@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-improvement Pull request with some product improvements label Jul 1, 2022
@CurtizJ CurtizJ self-assigned this Jul 3, 2022
Comment on lines +20 to +21
intermediate_header = getOutputPort().getHeader();
intermediate_header.erase(0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, explain why first column is removed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

First column is __grouping_set. It's added exactly before returning generated chunk. intermediate_heade is used to correctly create columns with defaults for input blocks (they don't have __grouping_set column).

@CurtizJ
Copy link
Copy Markdown
Member

CurtizJ commented Jul 8, 2022

Stress test failures are related:

SELECT
    CounterID AS k,
    quantileBFloat16(0.5)(ResolutionWidth)
FROM remote('127.0.0.{1,2}', default, hits)
GROUP BY k
ORDER BY
    count() DESC,
    CounterID ASC
LIMIT 10
SETTINGS group_by_use_nulls = 1

Query id: 3efcd4cf-74fc-4e78-b39b-94d1227953c7

→ Progress: 8.87 million rows, 53.24 MB (83.89 million rows/s., 503.35 MB/s.)  99%
0 rows in set. Elapsed: 0.190 sec. Processed 8.87 million rows, 53.24 MB (46.81 million rows/s., 280.85 MB/s.)

Received exception from server (version 22.7.1):
Code: 49. DB::Exception: Received from localhost:9000. DB::Exception: Bad cast from type DB::ColumnNullable to DB::ColumnVector<unsigned int>. (LOGICAL_ERROR)

Failure with WindowView is probably related too.

@novikd novikd requested a review from CurtizJ July 13, 2022 11:41
@novikd
Copy link
Copy Markdown
Member Author

novikd commented Jul 20, 2022

Stateless tests (thread, actions) [3/3] — Some tests restarted, fail: 1, passed: 1396, skipped: 10

Seems unrelated

@novikd novikd merged commit a09422d into master Jul 20, 2022
@novikd novikd deleted the group-by-use-nulls branch July 20, 2022 12:42
@novikd novikd mentioned this pull request Jul 29, 2022
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A setting rollup_use_nulls for standard compatibility

4 participants