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

feat: Implement map_keys_by_top_n_values function in Velox #12209

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

duxiao1212
Copy link
Contributor

Summary:
Implement map_keys_by_top_n_values function in Velox ;

the function returns the top N keys of the given map in descending order according to the natural ordering of its values.

Differential Revision: D68812985

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 30, 2025
Copy link

netlify bot commented Jan 30, 2025

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit bcd84eb
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/679acb7d81d81e0008bfdd5d

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68812985

@duxiao1212 duxiao1212 changed the title Implement map_keys_by_top_n_values function in Velox Feat: Implement map_keys_by_top_n_values function in Velox Jan 30, 2025
…or#12209)

Summary:

Implement map_keys_by_top_n_values function in Velox ;

the function  returns the top N keys of the given map in descending order according to the natural ordering of its values.

Differential Revision: D68812985
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68812985

@duxiao1212 duxiao1212 changed the title Feat: Implement map_keys_by_top_n_values function in Velox feat: Implement map_keys_by_top_n_values function in Velox Jan 30, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68812985

const size_t ttlSize =
std::min(static_cast<size_t>(n), static_cast<size_t>(inputMap.size()));

std::vector<It> container;
Copy link
Contributor

Choose a reason for hiding this comment

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

curious why do we need this container and another result?

@zacw7
Copy link
Contributor

zacw7 commented Jan 30, 2025

Could you please add the doc for this function here: https://github.com/facebookincubator/velox/blob/main/velox/docs/functions/presto/map.rst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants