feat(spanner): add support for handling null value in Proto columns by harshachinta · Pull Request #6954 · googleapis/google-cloud-go

harshachinta · 2022-10-27T19:21:11Z

This PR contains implementation changes to add support for handling null values in Proto Messages and Proto Enum type columns. Also the PR has changes to get protoTypeFqn for nil types.

spanner/value_test.go

Shua1 · 2022-10-28T08:37:58Z

spanner/value.go

+}
+
+// MarshalJSON implements json.Marshaler.MarshalJSON for NullProto.
+func (n NullProto) MarshalJSON() ([]byte, error) {


Could you please help me understand when and how MarshalJSON is used?

MarshalJSON() converts a custom type (in this case NullProto) defined in client library to a []byte. These are few methods that are exposed to users and it is followed as a convention for every custom type in Go client library.

How is the user expected to use the returned []byte? Is it for debugging only?

@Shua1

Could you please help me understand when and how MarshalJSON is used?

We implement the MarshalJSON() and UnmarshalJSON() methods of Marshaler and Unmarshaler interfaces that belongs to the json package in golang. This is to provide the functionality of encoding custom types NullProtoMessage and NullProtoEnum into a JSON string and decoding JSON string into NullProtoMessage and NullProtoEnum.

How is the user expected to use the returned []byte?

Usage:
json.Marshal method can be used to encode a NullProtoMessage and NullProtoEnum into a JSON string.
json.Unmarshal method can be used to decode a JSON string into NullProtoMessage and NullProtoEnum.

As NullProtoMessage and NullProtoEnum are custom types we have to implement the above interfaces for the encode and decode to work as expected.

Added tests for these methods.

Why do we need to encode and decode json?

spanner/value.go

olavloite

I'm not familiar enough with proto columns to know whether all of this makes sense or not, so I will have to defer to others for that. I've left a couple of more generic comments.

spanner/value.go

Shua1 · 2022-11-03T03:56:07Z

spanner/value.go

+}
+
+// MarshalJSON implements json.Marshaler.MarshalJSON for NullProto.
+func (n NullProto) MarshalJSON() ([]byte, error) {


How is the user expected to use the returned []byte? Is it for debugging only?

spanner/value.go

spanner/integration_test.go

Shua1

It's a bit hard to imagine the impact of this change on how users would use it. Let's proceed to see the dedicated integration tests and samples. Do we have issue tracking for them?

olavloite

LGTM, with some minor nits on comments and an error message

spanner/value.go

harshachinta · 2022-11-10T04:13:30Z

It's a bit hard to imagine the impact of this change on how users would use it. Let's proceed to see the dedicated integration tests and samples. Do we have issue tracking for them?

As golang maintains a seperate repository for samples and its integration tests, I will be raising a PR for the dedicated samples and integration tests for proto columns. The draft PR for these changes can be found here - PR-2754

* feat(spanner): add support for Proto Columns (#6886) * feat(spanner): Adding support for Proto Message and Proto Enum * feat(spanner): Add license header to new files * code refactoring and additional checks * nit: code changes * go.mod update to prevent failing builds * go.mod update to prevent failing builds * go.mod and build_samples.sh update to prevent failing builds * revert back grpc version * revert back changes * Using standard Singers example, refactoring redundant code * code and proto refactoring * Add proto_type_fqn for Proto Message and Proto Enum * code refactoring * go mod tidy: go.mod and go.sum version updates * add changes to support compatibility between Int64 and Enum & compatibility between Bytes and Proto * Revert "go mod tidy: go.mod and go.sum version updates" This reverts commit 484b00c. * add Integration Tests for Proto Message, Proto Enum, compatibility tests * code refactoring * code refactoring * add unit tests for nil proto types * Add error when nil proto message or nil enum is passed * feat(spanner): add support for handling null value in Proto columns (#6954) * feat(spanner): add support for handling null value in Proto columns * code refactor: get protoTypeFqn from user defined nil types * code refactoring * code refactoring * code refactoring * code refactoring * Add tests for MarshalJSON and UnmarshalJSON methods * refactoring test file * code refactoring * feat(spanner): add support for Array of Proto columns (#7014) * feat(spanner): add support for array of proto columns * refactoring comments and added negative test cases while reading array of protos * change decoding logic of handling array of proto columns * feat(spanner): add support for handling null values in array of proto columns (#7042) * feat(spanner): add support for handling null values in Array of Proto Columns * add comments for code readability * nit: change in error message * feat(spanner): Modify configuration for integration test and add license header (#7059) * feat(spanner): update go-genproto dependency (#7066) * feat(spanner): support for enum columns as keys, index and integration tests (#7091) * feat(spanner): support for proto columns as primary key and tests for parameterized queries, primary key and indexes * feat(spanner): close read-only transaction to prevent session leak * feat(spanner): update table schema to have GPK on proto field * feat(spanner): add proto changes to support proto columns feature * feat(spanner): remove gen-proto dep overwrite for proto column support * feat(spanner): remove gen-proto dep overwrite from kokoro build * feat(spanner): reset array of proto, enum when null from database (#7176) * feat(spanner): reset array of proto, enum when null from database * feat(spanner): fix license header to fix vet.sh build * feat(spanner): fix proto generated file to fix vet.sh build * feat(spanner): organize imports to fix vet.sh build * feat(spanner): golint changes to fix vet.sh build * feat(spanner): add support for Proto column DDL (#7292) * feat(spanner): add proto changes for Proto Columns DDL support * feat(spanner): add Proto descriptor file and integration tests for Proto DDL feature * feat(spanner): skip pg tests and code refactoring for proto ddl * feat(spanner): rename NewDatabaseAdminRESTClient to NewDatabaseAdminClient due to visibility label issue fix * feat(spanner): remove hardcoded cloud-devel host * feat(spanner): remove hardcoded project id * feat(spanner): revert autogenerated code * feat(spanner): just to validate integration tests running * feat(spanner): change copyright year * feat(spanner): set project id for integration tests * feat(spanner): use jsoniter instead of json for marshal and unmarshal * feat(spanner): revert presubmit.sh changes * chore(spanner): run integration tests in presubmit * chore(spanner): revert presubmit.sh changes * feat(spanner): fix json --------- Co-authored-by: rahul2393 <irahul@google.com>

feat(spanner): add support for handling null value in Proto columns

1db1052

harshachinta requested review from a team, gauravpurohit06, olavloite and rahul2393 October 27, 2022 19:21

product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the Spanner API. labels Oct 27, 2022

Shua1 reviewed Oct 28, 2022

View reviewed changes

code refactor: get protoTypeFqn from user defined nil types

9eb407c

olavloite reviewed Nov 1, 2022

View reviewed changes

spanner/value.go Show resolved Hide resolved

spanner/value.go Outdated Show resolved Hide resolved

spanner/value.go Outdated Show resolved Hide resolved

spanner/value.go Show resolved Hide resolved

spanner/value.go Outdated Show resolved Hide resolved

code refactoring

2cce43b

harshachinta requested review from Shua1 and olavloite November 1, 2022 13:33

Shua1 reviewed Nov 3, 2022

View reviewed changes

harshachinta added 2 commits November 3, 2022 09:27

code refactoring

2441b1d

code refactoring

037096b

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Nov 3, 2022

harshachinta added 3 commits November 3, 2022 12:55

code refactoring

efffa63

Add tests for MarshalJSON and UnmarshalJSON methods

5df6e6c

refactoring test file

71c1754

harshachinta requested a review from Shua1 November 3, 2022 17:39

rahul2393 approved these changes Nov 4, 2022

View reviewed changes

Shua1 approved these changes Nov 5, 2022

View reviewed changes

olavloite approved these changes Nov 7, 2022

View reviewed changes

spanner/value.go Show resolved Hide resolved

spanner/value.go Outdated Show resolved Hide resolved

spanner/value.go Show resolved Hide resolved

code refactoring

ab81750

harshachinta closed this Nov 8, 2022

harshachinta reopened this Nov 8, 2022

harshachinta merged commit 84b58d4 into googleapis:proto-column-enhancement-alpha Nov 10, 2022

harshachinta mentioned this pull request Nov 11, 2022

feat(spanner): add support for Array of Proto columns #7014

Merged

harshachinta mentioned this pull request Jan 29, 2024

feat(spanner): add support for Proto Columns #9315

Merged

Conversation

harshachinta commented Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Shua1 Oct 28, 2022

Choose a reason for hiding this comment

Uh oh!

harshachinta Oct 28, 2022

Choose a reason for hiding this comment

Uh oh!

Shua1 Nov 3, 2022

Choose a reason for hiding this comment

Uh oh!

harshachinta Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shua1 Nov 4, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

olavloite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Shua1 Nov 3, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Shua1 left a comment

Choose a reason for hiding this comment

Uh oh!

olavloite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harshachinta commented Nov 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

harshachinta commented Oct 27, 2022 •

edited

Loading

harshachinta Nov 3, 2022 •

edited

Loading