Skip to content

Conversation

pdet
Copy link
Contributor

@pdet pdet commented Jun 2, 2025

This also simplifies the destruction code, since these strings will be cleaned up with the ArgMinMaxStateBase

@pdet pdet changed the base branch from main to v1.3-ossivalis June 2, 2025 11:54
Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Aggregate states are pretty space-critical given they are instantiated per row in the aggregate hash table (which can be many). I'm not a big fan of expanding the aggregate state size here - can we fix this in a different way?

@pdet pdet marked this pull request as draft June 3, 2025 16:16
@pdet pdet marked this pull request as ready for review June 3, 2025 16:16
@duckdb-draftbot duckdb-draftbot marked this pull request as draft June 3, 2025 18:43
@pdet pdet marked this pull request as ready for review June 4, 2025 08:25
@Mytherin Mytherin merged commit 4d90750 into duckdb:v1.3-ossivalis Jun 4, 2025
101 checks passed
@Mytherin
Copy link
Collaborator

Mytherin commented Jun 4, 2025

Thanks!

github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Jun 5, 2025
Storge the argument and value of arg_min_max in the state as a unique_ptr (duckdb/duckdb#17749)
github-actions bot added a commit to duckdb/duckdb-r that referenced this pull request Jun 5, 2025
Storge the argument and value of arg_min_max in the state as a unique_ptr (duckdb/duckdb#17749)

Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
@pdet pdet deleted the arg_str branch July 18, 2025 12:48
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.

2 participants