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

Bg unsafe2 #7446

Closed
wants to merge 5 commits into from
Closed

Bg unsafe2 #7446

wants to merge 5 commits into from

Conversation

avantgardnerio
Copy link
Contributor

Which issue does this PR close?

Closes #7191.

Rationale for this change

PR #7192 could be faster.

What changes are included in this PR?

Convert the Vec<> heap into a RawPointer one, so that the map never needs to be updated. I intend to just close this PR for now, but I wanted it on record in case we ever decide to fully optimize this.

Are these changes tested?

No

Are there any user-facing changes?

No

@github-actions github-actions bot added optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) substrait labels Aug 30, 2023
@avantgardnerio
Copy link
Contributor Author

Let's reopen if constantly updating the map with new heap indexes becomes to burdensome / expensive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) substrait
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory is coupled to group by cardinality, even when the aggregate output is truncated by a limit clause
1 participant