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

[Go] Extension Builder Interface #34453

Closed
yevgenypats opened this issue Mar 4, 2023 · 0 comments · Fixed by #34454
Closed

[Go] Extension Builder Interface #34453

yevgenypats opened this issue Mar 4, 2023 · 0 comments · Fixed by #34454

Comments

@yevgenypats
Copy link
Contributor

Describe the enhancement requested

(This was also send in the mailing list and discussed shortly with @zeroshade ). Copy of what was sent in the mailing list and a PR will quickly follow:

Hopefully this is the right place to ask. As some background I'm Yevgeny Pats, Founder @ CloudQuery . We are very interested in migrating our protocol and Go type system to Apache Arrow. Extensions are a critical part for us and thus I've the following questions on whether it's a usage problem on my end or something that is not yet available. I'll give here an example for Go but I believe the same issue exists in all libraries/languages.

Here is a public github gist.

What are the problems:

  • The problems are around the abstraction for the extension types. While I understand that the underlying storage needs to be supported in the library we don't have a way for extensions to provide its own builder which means the user needs to know how the extension type stores the type inside the binary. This creates a leaky abstraction and the need for various helper functions like UUIDToBinary
  • The other way is fine as you can have methods like ToUUID on top of the extension array. But this creates asymmetry in the abstraction.
  • Because we don't control the builder for extensions this cripples into other places like json and csv where we can't control marshalling (in the same way we control all other built-in types). So basically for extensions that use binary type as underlying storage in case of json and csv those will always be encoded as base64 which is not very useful (think about uuid, ip address, mac address).

The main point is that I think the right abstraction for extensions should provide all the apis (type, array, builder) just like built-in types, otherwise the abstraction is incomplete or "leaky". Of course we can still have limitations like the custom builder must use an underlying known storage (for it to work over ipc) but it can still control various other types like marshaling, unmarshaling, building, and so on.

Hopefully this gives enough context but would love to elaborate.

Thanks,
Yevgeny

Component(s)

Go

yevgenypats added a commit to yevgenypats/arrow that referenced this issue Mar 4, 2023
@kou kou changed the title [GO] Extension Builder Interface [Go] Extension Builder Interface Mar 4, 2023
yevgenypats added a commit to yevgenypats/arrow that referenced this issue Mar 8, 2023
yevgenypats added a commit to yevgenypats/arrow that referenced this issue Mar 9, 2023
yevgenypats added a commit to yevgenypats/arrow that referenced this issue Mar 9, 2023
@zeroshade zeroshade added this to the 12.0.0 milestone Mar 10, 2023
zeroshade pushed a commit that referenced this issue Mar 10, 2023
This should serve as discussion as it's a medium change but this should Close #34453 and give users the ability to define custom Builder for their extensions just like they define ExtensionTypes and ExtensionArrays
* Closes: #34453

Authored-by: Yevgeny Pats <16490766+yevgenypats@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
yevgenypats added a commit to cloudquery/arrow that referenced this issue Mar 16, 2023
yevgenypats added a commit to cloudquery/arrow that referenced this issue Mar 17, 2023
zeroshade pushed a commit that referenced this issue Mar 23, 2023
Built on top of #34453

### Rationale for this change

### What changes are included in this PR?

### Are these changes tested?

### Are there any user-facing changes?

* Closes: #34584

Authored-by: Yevgeny Pats <16490766+yevgenypats@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this issue Mar 27, 2023
Built on top of apache#34453

### Rationale for this change

### What changes are included in this PR?

### Are these changes tested?

### Are there any user-facing changes?

* Closes: apache#34584

Authored-by: Yevgeny Pats <16490766+yevgenypats@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this issue May 15, 2023
Built on top of apache#34453

### Rationale for this change

### What changes are included in this PR?

### Are these changes tested?

### Are there any user-facing changes?

* Closes: apache#34584

Authored-by: Yevgeny Pats <16490766+yevgenypats@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
kou pushed a commit to apache/arrow-go that referenced this issue Aug 30, 2024
This should serve as discussion as it's a medium change but this should Close apache/arrow#34453 and give users the ability to define custom Builder for their extensions just like they define ExtensionTypes and ExtensionArrays
* Closes: #34453

Authored-by: Yevgeny Pats <16490766+yevgenypats@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
kou pushed a commit to apache/arrow-go that referenced this issue Aug 30, 2024
Built on top of apache/arrow#34453

### Rationale for this change

### What changes are included in this PR?

### Are these changes tested?

### Are there any user-facing changes?

* Closes: #34584

Authored-by: Yevgeny Pats <16490766+yevgenypats@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants