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

ARROW-8226: [Go] Implement 64 bit offsets binary builder #6725

Closed
wants to merge 14 commits into from

Conversation

richardartoul
Copy link
Contributor

No description provided.

richardartoul and others added 7 commits March 9, 2020 09:48
Co-authored-by: Richard Artoul <richard.artoul@datadoghq.com>
The checkptr check in go 1.14 flags `unsafe.Pointer(uintptr(x))` as potentially unsafe due to the possibility of dereferencing arbitrary memory.  This commit instead uses bare pointers for the length & byte fields
@github-actions
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW

Then could you also rename pull request title in the following format?

ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@richardartoul richardartoul changed the title [Go] Implement 64 bit offsets binary builder ARROW-8226: [Go] Implement 64 bit offsets binary builder Mar 26, 2020
@github-actions
Copy link

@richardartoul
Copy link
Contributor Author

@sbinet this is the new one

@richardartoul
Copy link
Contributor Author

@wesm Mind taking a look at this?

@emkornfield
Copy link
Contributor

ping @sbinet do you think you will have time to take a look?

@nealrichardson
Copy link
Contributor

Can we merge this?

@wesm
Copy link
Member

wesm commented Jun 24, 2020

Hm, at a high level it seems like it might be better to have a separate set of LargeBinary types rather than try to pack both 32-bit and 64-bit into the same types. This means some code duplication (given Go's current generics/templates situation) but perhaps for the best

@wesm
Copy link
Member

wesm commented Jun 24, 2020

@sbinet could you assist with this?

@richardartoul
Copy link
Contributor Author

Just wanted to share that I probably can't contribute any more time to getting this merged, we've moved away from Arrow for our project (we still use the format in some cases, but its our own implementation of builders) at this. Nothing to do with the project itself, we just needed more control over allocations and serialization and it was easier for us if implemented it all ourselves

@wesm
Copy link
Member

wesm commented Jun 26, 2020

Well, that is definitely a bummer. I hope to see some growth in the Go developer community here in the future. I'll close this PR for now then

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.

None yet

6 participants