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

Support nullable group by #4340

Merged
merged 1 commit into from
Mar 6, 2022
Merged

Conversation

sundy-li
Copy link
Member

@sundy-li sundy-li commented Mar 5, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Improve performance for group by nullable column

Changelog

  • Improvement

Related Issues

Continue: #4158
Fixes #4079

Test Plan

Unit Tests

Stateless Tests

@vercel
Copy link

vercel bot commented Mar 5, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/databend/databend/FFe46UQMJwUEU4LL2gTvA6ZG1yJg
✅ Preview: Canceled

@sundy-li sundy-li requested review from zhang2014 and zhyass and removed request for BohuTANG March 5, 2022 23:42
@mergify
Copy link
Contributor

mergify bot commented Mar 5, 2022

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@@ -39,15 +73,40 @@ impl Series {
/// Apply binary mode function to each element of the column.
/// WARN: Can't use `&mut [Vec<u8>]` because it has performance drawback.
/// Refer: https://github.com/rust-lang/rust-clippy/issues/8334
pub fn serialize(column: &ColumnRef, vec: &mut Vec<Vec<u8>>) -> Result<()> {
/*
* not nullable col1 nullable col2 (first byte to indicate null or not)
Copy link
Member

Choose a reason for hiding this comment

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

💯

@sundy-li
Copy link
Member Author

sundy-li commented Mar 6, 2022

Ontime datasets benchmark (Both run on the second query with metacache):

Before:

MySQL [(none)]> SELECT DayOfWeek, count(*) AS c FROM ontime GROUP BY DayOfWeek ORDER BY c DESC;
+-----------+----------+
| DayOfWeek | c        |
+-----------+----------+
|         3 | 23513895 |
|         2 | 23384295 |
|         4 | 23113350 |
|         7 | 21270195 |
|         1 | 20861550 |
|         6 | 19360215 |
|         5 | 18896085 |
+-----------+----------+
7 rows in set (2.578 sec)

After:

MySQL [(none)]> SELECT DayOfWeek, count(*) AS c FROM ontime GROUP BY DayOfWeek ORDER BY c DESC;
+-----------+----------+
| DayOfWeek | c        |
+-----------+----------+
|         3 | 23513895 |
|         2 | 23384295 |
|         4 | 23113350 |
|         7 | 21270195 |
|         1 | 20861550 |
|         6 | 19360215 |
|         5 | 18896085 |
+-----------+----------+
7 rows in set (0.896 sec)

@BohuTANG BohuTANG merged commit 2c42427 into databendlabs:main Mar 6, 2022
@BohuTANG
Copy link
Member

BohuTANG commented Mar 6, 2022

Also cc @wubx @dantengsky for the performance improvement.

It's time to change the subject/ontime table schema:
#4342

@wangzhen11aaa
Copy link
Contributor

C~l

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: full support of group by nullable column
5 participants