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.

Sign up for GitHub

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

Jump to bottom

[Go] FixedSizeBinary Marshal/Unmarshal returns error #35027

Closed
hermanschaaf opened this issue Apr 11, 2023 · 0 comments · Fixed by #35028
Closed

[Go] FixedSizeBinary Marshal/Unmarshal returns error #35027

hermanschaaf opened this issue Apr 11, 2023 · 0 comments · Fixed by #35028
Assignees
@hermanschaaf
Labels
Component: Go Type: bug
Milestone
13.0.0

Comments

@hermanschaaf
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

When using MarshalJSON on a FixedSizeBinary, I would expect that calling UnmarshalJSON on a FixedSizeBinaryBuilder with the produced JSON would read the original rows back into the builder. However right now these operations are not symmetrical, and an error is returned instead:

failed to unmarshal json: illegal base64 data at input byte 6

This is because the FixedSizeBinaryBuilder is using base64.RawStdEncoding while the json.Marshal method is using base64.StdEncoding. It seems like we should change the FixedSizeBinaryBuilder implementation to match the json.Marshal call and also use base64.StdEncoding.

I will open a pull request to fix this shortly.

Component(s)

Go
@github-actions github-actions bot mentioned this issue Apr 11, 2023
Merged
@zeroshade zeroshade added this to the 13.0.0 milestone Apr 11, 2023
zeroshade pushed a commit that referenced this issue Apr 14, 2023
@hermanschaaf @disq
GH-35027: [Go]: Use base64.StdEncoding in FixedSizeBinaryBuilder Unma…
3f5457d 
…rshal (#35028)

This changes `FixedSizeBinaryBuilder`'s Unmarshal to use `base64.StdEncoding` instead of `base64.RawStdEncoding`. The previous implementation caused an error when unmarshaling from JSON previously produced by a `FixedSizeBinary` array instance.

I also added a test for Marshal/Unmarshal to set the expectation that these operations should mirror one another.
* Closes: #35027

Lead-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Kemal Hadimli <disq@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
liujiacheng777 pushed a commit to LoongArch-Python/arrow that referenced this issue May 11, 2023
@hermanschaaf @disq @liujiacheng777
apacheGH-35027: [Go]: Use base64.StdEncoding in FixedSizeBinaryBuilde…
5089304 
…r Unmarshal (apache#35028)

This changes `FixedSizeBinaryBuilder`'s Unmarshal to use `base64.StdEncoding` instead of `base64.RawStdEncoding`. The previous implementation caused an error when unmarshaling from JSON previously produced by a `FixedSizeBinary` array instance.

I also added a test for Marshal/Unmarshal to set the expectation that these operations should mirror one another.
* Closes: apache#35027

Lead-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Kemal Hadimli <disq@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
@hermanschaaf @disq @ArgusLi
apacheGH-35027: [Go]: Use base64.StdEncoding in FixedSizeBinaryBuilde…
0b97c9c 
…r Unmarshal (apache#35028)

This changes `FixedSizeBinaryBuilder`'s Unmarshal to use `base64.StdEncoding` instead of `base64.RawStdEncoding`. The previous implementation caused an error when unmarshaling from JSON previously produced by a `FixedSizeBinary` array instance.

I also added a test for Marshal/Unmarshal to set the expectation that these operations should mirror one another.
* Closes: apache#35027

Lead-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Kemal Hadimli <disq@users.noreply.github.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this issue May 16, 2023
@hermanschaaf @disq @rtpsw
apacheGH-35027: [Go]: Use base64.StdEncoding in FixedSizeBinaryBuilde…
a527c7e 
…r Unmarshal (apache#35028)

This changes `FixedSizeBinaryBuilder`'s Unmarshal to use `base64.StdEncoding` instead of `base64.RawStdEncoding`. The previous implementation caused an error when unmarshaling from JSON previously produced by a `FixedSizeBinary` array instance.

I also added a test for Marshal/Unmarshal to set the expectation that these operations should mirror one another.
* Closes: apache#35027

Lead-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Kemal Hadimli <disq@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
Assignees

@hermanschaaf hermanschaaf

Labels
Component: Go Type: bug
Projects
None yet
Milestone
13.0.0
Development

Successfully merging a pull request may close this issue.

GH-35027: [Go]: Use base64.StdEncoding in FixedSizeBinaryBuilder Unmarshal cloudquery/arrow
2 participants
@zeroshade @hermanschaaf