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

refactor: Simplify the partial full handling for distinct aggregation #11876

Closed
wants to merge 1 commit into from

Conversation

xiaoxmeng
Copy link
Contributor

Summary:
The current partial full is cleared when there is no new distinct in add input if it has been set.
The reason for doing this is to avoid distinct partial aggregation hanging problem
as we only reset the grouping set in case of partial full if there is new distinct found in get output.
This is a bit tricky and it is better to handle reset partial full unconditionally in get output for
distinct type of aggregation.

Differential Revision: D67288365

@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 Dec 16, 2024
@facebook-github-bot
Copy link
Contributor

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

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 108cc0b
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6760cfdb58eb2b0008fffa7f

xiaoxmeng added a commit to xiaoxmeng/velox that referenced this pull request Dec 17, 2024
…facebookincubator#11876)

Summary:

The current partial full is cleared when there is no new distinct in add input if it has been set.
The reason for doing this is to avoid distinct partial aggregation hanging problem
as we only reset the grouping set in case of partial full if there is new distinct found in get output.
This is a bit tricky and it is better to handle reset partial full unconditionally in get output for
distinct type of aggregation.

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

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

…facebookincubator#11876)

Summary:

We should have the case in aggregation add input that the partial aggregation has been marked as full
but there is no new distinct detected for a distinct aggregation operator. Instead of clearing the partial
full flag, we should add a distinct check there instead for simplicity.

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

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

@xiaoxmeng xiaoxmeng closed this Dec 19, 2024
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.

2 participants