Skip to content

GH-20407: [Go] Array Builder for REE arrays - #14114

Merged
zeroshade merged 7 commits into
apache:masterfrom
zeroshade:rle-array-builder
Jan 23, 2023
Merged

GH-20407: [Go] Array Builder for REE arrays#14114
zeroshade merged 7 commits into
apache:masterfrom
zeroshade:rle-array-builder

Conversation

@zeroshade

@zeroshade zeroshade commented Sep 13, 2022 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

@github-actions

github-actions Bot commented Sep 13, 2022

Copy link
Copy Markdown

https://issues.apache.org/jira/browse/ARROW-17709

@zeroshade
zeroshade force-pushed the rle-array-builder branch 3 times, most recently from addd2c6 to cff0e08 Compare October 21, 2022 15:58
@asfimport asfimport mentioned this pull request Oct 20, 2022
Closed
@zeroshade zeroshade changed the title ARROW-17709: [Go] Array Builder for RLE arrays ARROW-17709: [Go] Array Builder for REE arrays Jan 17, 2023
@zeroshade
zeroshade marked this pull request as ready for review January 17, 2023 19:56
@zeroshade
zeroshade requested a review from lidavidm January 17, 2023 19:57
lidavidm
lidavidm reviewed Jan 18, 2023
View reviewed changes

@lidavidm lidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

Comment thread go/arrow/array/encoded.go Outdated

@lidavidm lidavidm Jan 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be useful to have an AppendNulls as well?

@lidavidm lidavidm Jan 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose you can always do that in terms of ValueBuilder and Append though

@zeroshade zeroshade Jan 18, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yea, if i'm gonna add AppendNulls then I should add it to all the builders and make it part of the interface and i don't really wanna do that in this PR haha.

@zeroshade zeroshade changed the title ARROW-17709: [Go] Array Builder for REE arrays GH-20407: [Go] Array Builder for REE arrays Jan 18, 2023
@github-actions

github-actions Bot commented Jan 18, 2023

Copy link
Copy Markdown

@zeroshade

zeroshade commented Jan 23, 2023

Copy link
Copy Markdown
Member Author

Gonna give this till EOD today for anyone else to comment on it, barring any comments I'll merge it tonight.

@westonpace
westonpace removed their request for review January 23, 2023 18:48
@zeroshade
zeroshade merged commit 408e9ce into apache:master Jan 23, 2023
@zeroshade
zeroshade deleted the rle-array-builder branch January 23, 2023 23:07
@ursabot

ursabot commented Jan 24, 2023

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 8449c55 and contender = 408e9ce. 408e9ce is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️1.49% ⬆️0.03%] test-mac-arm
[Failed ⬇️0.46% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.16% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 408e9ce1 ec2-t3-xlarge-us-east-2
[Failed] 408e9ce1 test-mac-arm
[Failed] 408e9ce1 ursa-i9-9960x
[Finished] 408e9ce1 ursa-thinkcentre-m75q
[Finished] 8449c553 ec2-t3-xlarge-us-east-2
[Failed] 8449c553 test-mac-arm
[Failed] 8449c553 ursa-i9-9960x
[Finished] 8449c553 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidavidm lidavidm lidavidm left review comments
@cyb70289 cyb70289 Awaiting requested review from cyb70289

Assignees

No one assigned

Labels

Component: Go

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Go] Array Builder for RLE Arrays

3 participants

@zeroshade @ursabot @lidavidm