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

Minor : Improve hash join build side recordbatch size accuracy #13916

Merged
merged 1 commit into from
Dec 29, 2024

Conversation

getChan
Copy link
Contributor

@getChan getChan commented Dec 27, 2024

Which issue does this PR close?

Part of #13430

Rationale for this change

In Hash Join build side, the RecordBatch size is overestimated.
I think that accurate memory accounting on the build side of hash join is important.

Fix it by using get_record_batch_memory_size.

What changes are included in this PR?

Are these changes tested?

Unit tested for a narrow range.

Are there any user-facing changes?

no

@github-actions github-actions bot added the physical-expr Physical Expressions label Dec 27, 2024

assert_contains!(
err.to_string(),
"Failed to allocate additional 120 bytes for HashJoinInput"
Copy link
Contributor Author

@getChan getChan Dec 27, 2024

Choose a reason for hiding this comment

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

before fix. 408

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @getChan

FYI @2010YOUY01 the project is working!

@alamb alamb changed the title Minor : Fix hash join build side recordbatch size accuracy Minor : Improve hash join build side recordbatch size accuracy Dec 27, 2024
@alamb alamb merged commit a47729c into apache:main Dec 29, 2024
28 checks passed
@alamb
Copy link
Contributor

alamb commented Dec 29, 2024

Thanks again @getChan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
physical-expr Physical Expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants