Fix int64 handling #19
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vvakame
commented
Jul 18, 2019
| @@ -36,13 +35,20 @@ func (c *JSONColumn) marshal(t *gspanner.Type, v *structpb.Value) (interface{}, | |||
| // See: https://godoc.org/google.golang.org/genproto/googleapis/spanner/v1#TypeCode | |||
| switch t.Code { | |||
| case gspanner.TypeCode_INT64: | |||
| // JavaScript's integral part is 53bit. see Number.MAX_SAFE_INTEGER. | |||
| return v.GetStringValue(), nil | |||
There was a problem hiding this comment.
Goのint64だとJSで処理可能な範囲を超えて数値が勝手に丸められたりするので文字列で送る必要がある。
値によってエンコード方法を変えると辛いと思うので一括で文字列に。
| if err != nil { | ||
| return nil, err | ||
| switch s { | ||
| case "NaN", "Infinity", "-Infinity": |
There was a problem hiding this comment.
この辺はドキュメントに従って書いてあるとおりにコード書いた感じ(実環境での動作確認はしていない
| case gspanner.TypeCode_INT64, gspanner.TypeCode_FLOAT64: | ||
| case gspanner.TypeCode_INT64: | ||
| o.Set("type", "string") | ||
| o.Set("format", "int64") |
There was a problem hiding this comment.
Stringと区別はしたいはずなので悩みつつこうした。
https://swagger.io/docs/specification/data-models/data-types/ を参考に、typeはstringにしformatでint64であることを主張してみる。
|
あれーCIオチてるなと思ったらmasterすら落ちとるやんけ…！ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #17
refs #18
メ社の内部利用で問題になっているやつ(該当リポジトリからリンクさせておきます
JSONで数値周りの仕様というのは決まっていないんだけど、JavaScript環境で動作する必要はあると思っています。
SpannerのドキュメントとJSONの仕様とJSON Schemaの仕様を突き合わせた結果、このPRの通りの変更を加えるのが妥当そうだな…という気持ちです。
インラインでコメントを入れていきます。