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.

Sign up for GitHub

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

Jump to bottom

[Fix] Don't initialize reference, constant, and parameter children in intermediate chunk #14254

Merged
merged 12 commits into from
Oct 11, 2024
Merged

[Fix] Don't initialize reference, constant, and parameter children in intermediate chunk #14254

merged 12 commits into from
Oct 11, 2024

Conversation

taniabogatsch
Copy link
Contributor

Fixes #9628

Mytherin
Mytherin reviewed Oct 7, 2024
View reviewed changes
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! Looks good - some comments:

src/execution/expression_executor/execute_operator.cpp Outdated Show resolved Hide resolved
src/execution/expression_executor_state.cpp Outdated Show resolved Hide resolved
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 7, 2024 15:31
@taniabogatsch taniabogatsch marked this pull request as ready for review October 7, 2024 15:53
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 8, 2024 11:53
@taniabogatsch taniabogatsch marked this pull request as ready for review October 8, 2024 11:53
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 9, 2024 08:41
@taniabogatsch taniabogatsch marked this pull request as ready for review October 9, 2024 08:42
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 9, 2024 12:43
@taniabogatsch taniabogatsch marked this pull request as ready for review October 9, 2024 12:51
Mytherin
Mytherin reviewed Oct 10, 2024
View reviewed changes
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! LGTM - one minor comment

src/execution/expression_executor.cpp Outdated Show resolved Hide resolved
@duckdb-draftbot duckdb-draftbot marked this pull request as draft October 10, 2024 10:52
@taniabogatsch taniabogatsch marked this pull request as ready for review October 10, 2024 10:55
@Mytherin Mytherin merged commit d0cf23e into duckdb:main Oct 11, 2024
44 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

@taniabogatsch taniabogatsch deleted the unnest branch October 11, 2024 12:02
github-actions bot pushed a commit to duckdb/duckdb-r that referenced this pull request Oct 16, 2024
@krlmlr
chore: Update vendored sources to duckdb/duckdb@d0cf23e
db9144d 
[Fix] Don't initialize reference, constant, and parameter children in intermediate chunk (duckdb/duckdb#14254)
@github-actions github-actions bot mentioned this pull request Oct 16, 2024
Merged
github-actions bot added a commit to duckdb/duckdb-r that referenced this pull request Oct 16, 2024
@github-actions @krlmlr
chore: Update vendored sources to duckdb/duckdb@d0cf23e (#498)
85550b0 
[Fix] Don't initialize reference, constant, and parameter children in intermediate chunk (duckdb/duckdb#14254)

Co-authored-by: krlmlr <krlmlr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mytherin Mytherin Mytherin left review comments

@lnkuiper lnkuiper Awaiting requested review from lnkuiper

Assignees
No one assigned
Labels
Ready To Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UNNEST is very slow on a struct with high number of keys over arrow data
2 participants
@taniabogatsch @Mytherin