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

Fix from base64.RawStdEncoding to base64.StdEncoding #123

Merged

Conversation

tkuchiki
Copy link
Contributor

It seems that the BYTES type in Cloud Spanner needs to be encoded as base64 with padding.

https://github.com/googleapis/googleapis/blob/master/google/spanner/v1/type.proto#L116-L118
https://github.com/googleapis/google-cloud-go/blob/7eaaa470fda5dc7cd1ff041d6a898e35fb54920e/spanner/value.go#L1085

Currently, spanner-cli uses base64.RawStdEncoding, but the padding is missing when outputting the data.

Example of tests

スクリーンショット 2022-01-25 10 06 57

スクリーンショット 2022-01-25 10 06 46

Currently

$ spanner-cli -p <my-project> -i <my-instance> -d <my-db> -e 'SELECT id, bytes FROM TestTable'
id      bytes
1       aGVsbG8sIHdvcmxkIQ

Fixed

$ spanner-cli -p <my-project> -i <my-instance> -d <my-db> -e 'SELECT id, bytes FROM TestTable'
id      bytes
1       aGVsbG8sIHdvcmxkIQ==

@tkuchiki tkuchiki marked this pull request as ready for review January 25, 2022 01:25
@yfuruyama
Copy link
Collaborator

Thanks for fixing this!

Can you add a new test or fix this test so that we can check the padding?

@tkuchiki
Copy link
Contributor Author

tkuchiki commented Jan 25, 2022

Thank you for your review!

OK, I will add a new fix to the test.

@tkuchiki
Copy link
Contributor Author

I'm sorry for the late.
Fixed!

@yfuruyama yfuruyama self-requested a review January 25, 2022 13:06
@yfuruyama
Copy link
Collaborator

Thanks for the fix! LGTM!

@yfuruyama yfuruyama merged commit fe8624d into cloudspannerecosystem:master Jan 25, 2022
@tkuchiki tkuchiki deleted the use-base64-stdencoding branch January 26, 2022 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants