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
Add mode fot topK/topKWeighed function to also include count/error statistics #54508
Add mode fot topK/topKWeighed function to also include count/error statistics #54508
Conversation
This is an automated comment for commit 912b8ba with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good but it would be nice to add some tests
docs/en/sql-reference/aggregate-functions/reference/topkweighted.md
Outdated
Show resolved
Hide resolved
It was more like POC example, will work on tests, if it's OK change. Introduction of new aggregate function, like Anyway, it looks like, we better add APPROX_TOP_K (TopK) and APPROX_TOP_SUM (TopKWeighed) aliases, because they are much more common in other dbms. Unfortunately, they don't have concept of parameters, so everything is being passed as arguments, so different signature, but ClickHouse way is slightly better to my taste.
|
48b33ae
to
6910cdb
Compare
@UnamedRus could you please merge master again? |
Changelog category (leave one):
Fix for #42029
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
topK/topKWeighed support mode, which return count of values and it's error.
Documentation entry for user-facing changes
Parameter naming & handling can be better, i guess.