Skip to content

Improve ColumnarRow #16962

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pettyjamesm
Copy link
Contributor

Extracted partially from trinodb/trino#9916

  • Adds ColumnarRow#mayHaveNull() and uses it inside of hive's StructCoercer
  • Removes an unnecessary field array copy, all usage sites are private to the class and do not require a defensive clone
  • Adds special case for creating ColumnarRow instances from DictionaryBlocks that have no nulls
== NO RELEASE NOTE ==

Copy link
Contributor

@mbasmanova mbasmanova left a comment

Choose a reason for hiding this comment

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

@jamesgpearce Looks good to me. Have you observed some measurable performance wins? Do you know if the changes are fully covered by existing tests?

@pettyjamesm
Copy link
Contributor Author

Have you observed some measurable performance wins?

This is pending collection from our testing environment. I suspect it's a modest improvement, mostly from its usage in SpillableFinalOnlyGroupedAccumulator and from specific narrow usage scenarios involving the hive struct coercer.

Do you know if the changes are fully covered by existing tests?

Yeah, the existing TestColumnarRow covers RLE / Dictionary / plain block combinations with and without null rows exhaustively.

@stale
Copy link

stale bot commented Jun 13, 2022

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the task, make sure you've addressed reviewer comments, and rebase on the latest master. Thank you for your contributions!

@stale stale bot added the stale label Jun 13, 2022
@wanglinsong wanglinsong requested a review from a team as a code owner July 6, 2024 04:32
@wanglinsong wanglinsong requested a review from presto-oss July 6, 2024 04:32
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.

3 participants