feat: add support for JSON data type #872

zoercai · 2021-02-16T10:02:17Z

Allow users to read and write to Cloud Spanner databases using the JSON type through the client libraries.

Integration tests here: zoercai#1

generated-files-bot · 2021-02-16T10:07:50Z

Warning: This pull request is touching the following templated files:

proto-google-cloud-spanner-v1/src/main/java/com/google/spanner/v1/TypeCode.java
proto-google-cloud-spanner-v1/src/main/java/com/google/spanner/v1/TypeProto.java
proto-google-cloud-spanner-v1/src/main/proto/google/spanner/v1/type.proto

codecov · 2021-02-16T10:11:50Z

Codecov Report

Merging #872 (1147d8b) into master (cd45643) will increase coverage by 0.08%.
The diff coverage is 76.74%.

❗ Current head 1147d8b differs from pull request most recent head 3fe3112. Consider uploading reports for the commit 3fe3112 to get more accurate results

@@             Coverage Diff              @@
##             master     #872      +/-   ##
============================================
+ Coverage     84.86%   84.94%   +0.08%     
+ Complexity     2762     2753       -9     
============================================
  Files           156      156              
  Lines         14318    14502     +184     
  Branches       1377     1383       +6     
============================================
+ Hits          12151    12319     +168     
- Misses         1596     1614      +18     
+ Partials        571      569       -2

Impacted Files	Coverage Δ
...m/google/cloud/spanner/ForwardingStructReader.java	`22.52% <0.00%> (-1.75%)`	⬇️
...er/src/main/java/com/google/cloud/spanner/Key.java	`95.37% <ø> (ø)`
...in/java/com/google/cloud/spanner/StructReader.java	`0.00% <ø> (ø)`
...va/com/google/cloud/spanner/AbstractResultSet.java	`78.74% <50.00%> (-0.26%)`	⬇️
...src/main/java/com/google/cloud/spanner/Struct.java	`87.38% <50.00%> (-1.40%)`	⬇️
.../src/main/java/com/google/cloud/spanner/Value.java	`89.05% <72.72%> (-0.37%)`	⬇️
...com/google/cloud/spanner/AbstractStructReader.java	`98.51% <100.00%> (+0.11%)`	⬆️
...main/java/com/google/cloud/spanner/ResultSets.java	`95.32% <100.00%> (-1.77%)`	⬇️
...r/src/main/java/com/google/cloud/spanner/Type.java	`93.45% <100.00%> (+0.24%)`	⬆️
...ain/java/com/google/cloud/spanner/ValueBinder.java	`100.00% <100.00%> (ø)`
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffc190c...3fe3112. Read the comment docs.

skuruppu · 2021-02-17T02:55:34Z

@olavloite we're not sure why the InlineBeginTransactionWithoutExecutorTest is failing. Maybe a flake but not sure.

google-cloud-spanner/src/test/java/com/google/cloud/spanner/GrpcResultSetTest.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Json.java

thiagotnunes · 2021-02-17T03:47:42Z

@skuruppu @zoercai the failed integration test is a flake: #869

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Json.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Type.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ChecksumResultSet.java

google-cloud-spanner/src/test/java/com/google/cloud/spanner/GrpcResultSetTest.java

zoercai · 2021-04-12T08:07:19Z

Hi @olavloite I've removed the custom Json class as there were concerns with inconsistencies in the type of JSON documents that could be parsed by gson vs other languages' parsers. Now we are no longer binding the result value type to Json (we're just returning String), and for the input value type the user would need to specify to(Value.json("json")). This way only the backend controls the parsing and the normalization.
Could you please take another look and let me know if you think this approach could cause issues for any use cases?

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java

google-cloud-spanner/src/test/java/com/google/cloud/spanner/GrpcResultSetTest.java

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ReadFormatTestRunner.java

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java

olavloite

This looks good to me. I'll wait with the approve until the backend is also ready, so we can test it with the backend as well.

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java

thiagotnunes

LGTM, but I think we should not merge until we have some integration tests in place (when the backend is ready).

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java

zoercai · 2021-04-27T02:55:59Z

@thiagotnunes @olavloite Could you please take another look? I've added the integration tests here, they passed when I ran them locally against staging (since it's based on this branch, I couldn't assign you as reviewers)

olavloite · 2021-04-27T15:02:47Z

@thiagotnunes @olavloite Could you please take another look? I've added the integration tests here, they passed when I ran them locally against staging (since it's based on this branch, I couldn't assign you as reviewers)

@zoercai
I've added my review to the other PR.

thiagotnunes · 2021-04-27T22:36:17Z

@zoercai also added comments on the other PR

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java

thiagotnunes

I think it is a good idea for us to add JSON support in the MockSpannerServiceImpl as well. See https://github.com/googleapis/java-spanner/blob/master/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java#L1175

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractStructReader.java

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java

thiagotnunes

LGTM

* Feature: Support for Cloud Spanner This changeset introduces support for Google Cloud Spanner in the Elide model layer and ORM. Basic support for reading, writing, transactions, etc., are all planned. This is pretty easy, obviously, since there is already a Firestore adapter to work from. Fixes and closes sgammon/elide#856. * Tune bazel.rc and Makefile configs * Tweaks to build flow * Disable labs in CI * Promote strict_proto_deps / strict_system_includes * Whoops, fix labs state * Converge IntelliJ flags with Makefile * Add DDL-based Spanner table generator - Generate compliant DDL statements for creating Spanner tables from proto models - Add ability to stub/mock `FieldPointer` objects * Add concept of field size for Spanner columns * Significantly more driver testing - Add `TypeBuffet` to test conversion of different types - Add tests for `SpannerUtil` - Add initial tests for Spanner DDL generator - Uncomment more tests from `GenericPersistenceDriverTests` * Fixes for TypeScript-related tests * Further model adapter testing tweaks - Allow test caching (local only) - Enable other adapter/driver tests * Converge newer deps * Re-enable ancillary `rules_closure` tests - Fix CSS testing bugs - Re-enable style tests - Add suite targets for DOM/style tests * Enable trace logging in model adapter objects during tests * Refactor generic driver tests to allow opt-out pattern - Allow drivers to opt-out of tests they don't yet support - Enable all tests across the board * Enable more logging targets in testing * Add specialized Spanner tests to regular backend suite - Add `SpannerDDLTest` - Add `SpannerUtilTest` * Disallow transport config construction * Implement deletes in Spanner - Implement singular delete method - Enable standard driver `delete()` tests * Implement `FieldMask` enforcement in Spanner - Enforce `FieldMask` settings for fetched records - Drop superfluous error checking * Style and bug fixes from analysis tools * Wrap `InvalidProtocolBufferException` in `IllegalStateException` * Refactor error handling for embedded JSON models * Bugfixes for Spanner DDL generator - Fix bug with `ARRAY` fields missing inner types - Add sensible default for `ENUM` fields implemented as `STRING` * Better testing for DDL statements, fixes for `BYTES` fields - Enable column sizes for `BYTES` fields - Transition long string comparisons to Truth assertions * Hook tests into the new DDL generator - Generate tables for emulated testing - Add method alias to generate DDL statements just from a model instance (with default settings) * Drop branch coverage until logging settings don't bork it * Update Javadoc * Exclude docs/tests from CodeClimate Create CNAME Set theme jekyll-theme-cayman * Add missing package-info in gust/backend/annotations * Update javadoc * Use remote 'sleek' Jekyll theme * Drop empty method comment * Further CodeClimate config tweaks * Fix duplicate exclude_patterns in CodeClimate config * [skip-ci] License header updates * Drop docs from IDE project * Refactor `DatabaseManager` to bind adapter/driver - Bind adapter and driver together, that was the OG point - Add `Closeable`/`AutoCloseable` support to `SpannerManager` - Add full test coverage to `SpannerManager` - Add shared run profile for debuggable Bazel testsuite * Refresh docs * Don't remove docroot when cleaning docs * Disallow internal construction of `SpannerManagerSingleton` * Downgrade to Micronaut v2.5.4 * Feature: 'suspend_debug' when running apps * Support for `TIMESTAMP` and `DATE` types in Spanner - Support generating DDL for temporal column types - Codec support for converting between Spanner `DATE` and `com.google.type.Date` - Codec support for converting between Spanner `TIMESTAMP` and `com.google.protobuf.Timestamp` * Ungate serialization of `TIMESTAMP` and `DATE` types - Drop exceptions - Drop TODOs - Enable serialization of `TIMESTAMP` types from PB timestamps - Enable serialization of `DATE` types from PB common dates * Support properly encoding/decoding ENUMs in Spanner * Fix repeated field error in Spanner codec * Add dedicated manager spawn route with settings control The Spanner manager needs a way to spawn drivers with custom settings. It has a way to now. * Add ability to acquire Spanner client from manager * Add granular configuration to `SpannerManager` - Add ability to control executor - Add ability to control cache - Add ability to control base spanner options - Add ability to control main driver options * Add new Spanner tests to regular suite * Fix bug with key column names * Enforce singular KEY field state * Converge tests with new ID behavior * Resolve same key bug for generated DDL * Add enforcement for ID column types * Fix ID issues with default Spanner field sets * Fix issue with Integer vs. Long decoding in Spanner * Allow public acquisition of InMemoryCache instances * Provide 'resolveColumnName' alias with no settings * Open up 'SpannerStructDeserializer#forModel * Expose better streaming support for calculating column fields - Allow access to underlying field stream - Pair fields to corresponding column names - Maintain external API * Update docs * Fix missing license header * Fix issues with ID presence on returned models - Add tests to enforce ID presence on models returned via adapter fetch interfaces - Fix related bugs on Spanner adapter * [p0] Fix for debugger_support flag issue * Fix debug race conditions for JDK app targets * Better nullchecks for Spanner columns * Upgrades to Java dependencies - Upgrade GAX - Upgrade Micronaut - Upgrade Google Cloud SDK - Upgrade Firestore * Rollback GAX change * Fixes for various analyzer warnings * Implement transactional write/delete support - Set `transactionContext` on mutation options for buffered serialization to an active transaction - Acquire a transaction context via `TransactionManager.begin()` * Implement codec support for `NUMERIC` columns Following [Google's advice][1], I've implemented `NUMERIC` types as proto-strings. - Add deserializer support for `NUMERIC`s - Add serializer support for `NUMERIC`s - Add basic test for schema translation [1]: https://cloud.google.com/spanner/docs/working-with-numerics * Update docs * Implement support for ancillary Spanner annotations - Add `NONNULL` annotation - Add basic support for field expressions (including `STORED`) - Add `commit_update` support * Rebuild docs * Apply fixes for runtime jdeps * Bump BoringSSL/Netty -> '2.0.40.Final' * Netty/Micronaut version bumps * Upgrade rules_closure and protobuf -> 3.17.3 * Re-pin maven deps * Match gRPC's versions for Netty and BoringSSL * Re-upgrade Netty/TCNative * Force shaded version of Netty for gRPC * Add remote Bazel settings profile * Enable control of naming for Java image targets * Wire up container-based dev * General fixes for Bazel within dev containers * Dev container env adjustments * Add missing continuation slash * Add container init and post-attach commands * Add creds to codespaces image (via secrets) * Don't require env for dev container * Add default set of VSC extensions * Fix path access to ibazel/bazelisk in devcontainer init * Further adjustments to devcontainer env * Drop initialize command from devcontainer * Move container signal to env * Drop initialize command * Set container user to 'dev' * Add container-init script * Tweaks for init script safety * Force version v1d for codespaces * Upgrade devcontainer -> v1e * Improvements to container init script * Force container version v1f * Fixes for project-wide RBE settings * Upgrade container -> v1g * Upgrade to latest Codespaces container * Upgrade Bazel -> 4.2.0 * Apply lib overrides for Netty and Guava-JDK5 * Upgrade Bazel -> 4.2.1 * Add Micronaut Management dep * Update Spanner -> 6.13.0 * Upgrade GAX -> 2.5.0 * Add renovate.json * Switch Nonnull to NonNull (Micronaut) in AssetManager * Update Maven pins * Feature: Spanner JSON Columns This changeset adds support for Spanner's new native JSON columns, which essentially act as strings but with enforced JSON-friendly semantics which improve queryability. Spanner's JSON fields differ from strings in the following ways: - Keys are sorted lexicographically - Whitespace is not preserved - Duplicate keys are elided - Numeric and other primitive types are interpreted natively - JSON `null` is treated as SQL `NON NULL` These columns have advantages for various kinds of JSON document storage based in Spanner. See docs here: - [Spanner: Working with JSON](https://cloud.google.com/spanner/docs/working-with-json) - [Spanner/J: Driver support PR](googleapis/java-spanner#872) Changes enclosed: - Add support to DDL generator for JSON columns - Driver-side experimental flag to enable/disable - Compliant with existing testsuite (fallback to `STRING`) - Tests for tables with JSON columns - Tests for DDL generator with JSON columns enabled * Fix: Default `BOOL` values in Spanner This changeset adds a fix for sgammon/elide#868, wherein default `BOOL` column values should be the `false` value, rather than no value at all. This PR implements the approach described in the issue, gated behind a driver setting. Fixes and closes sgammon/elide#868. * Chore: General Deps Update This changeset updates some key dependencies that have issued newer releases since last pin. - [x] Update GAX -> `2.7.0` - [x] Update Google Cloud APIs -> `0.163.0` - [x] Update common protos -> `2.6.0` * Upgrade gRPC -> `1.42.0`, upgrade Protobuf -> `3.18.1` * Seal deps * Restore protobuf/gRPC versions * Drop unsupported configsets property from Graal binary builds * Restore protos, gRPC, GAX * Fixes for deps in rules_closure * Dependency tweaks (Proto/gRPC upgrade) * Fix for decoding of enums from Spanner string fields * Fix nasty Guava dependency alignment bug * Fixes for `DATE` vs. `TIMESTAMP` types in Spanner - Fallback to `TIMESTAMP` for `DATE` fields that error (seems to be a driver quirk) - Allow decoding spanner `DATE` fields into standard `google.protobuf.Timestamp` types Co-authored-by: Renovate Bot <[email protected]>

This is an auto-generated regeneration of the .pb.go files by cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will update the corresponding PR to depend on the newer version of go-genproto, and assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot will not create any more regeneration PRs. If all regen PRs are closed, gapicgen will create a new set of regeneration PRs once per night. If you have been assigned to review this PR, please: - Ensure that CI is passing. If it's failing, it requires your manual attention. - Approve and submit this PR if you believe it's ready to ship. That will prompt genbot to assign reviewers to the google-cloud-go PR. Corresponding google-cloud-go PR: googleapis/google-cloud-go#6473 Changes: feat(compute): Update Compute Engine API to revision 20220720 (googleapis#723) Source-Link: googleapis/googleapis@60a0fa7

zoercai added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 16, 2021

zoercai requested review from a team as code owners February 16, 2021 10:02

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 16, 2021

product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Feb 16, 2021

skuruppu requested a review from olavloite February 17, 2021 02:55

thiagotnunes reviewed Feb 17, 2021

View reviewed changes

google-cloud-spanner/src/test/java/com/google/cloud/spanner/GrpcResultSetTest.java Outdated Show resolved Hide resolved

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Json.java Outdated Show resolved Hide resolved

olavloite reviewed Feb 17, 2021

View reviewed changes

zoercai commented Apr 12, 2021

View reviewed changes

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java Outdated Show resolved Hide resolved

zoercai requested review from thiagotnunes and olavloite April 12, 2021 08:08

olavloite reviewed Apr 12, 2021

View reviewed changes

olavloite reviewed Apr 14, 2021

View reviewed changes

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java Outdated Show resolved Hide resolved

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java Outdated Show resolved Hide resolved

thiagotnunes reviewed Apr 15, 2021

View reviewed changes

google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java Outdated Show resolved Hide resolved

zoercai requested review from thiagotnunes and olavloite April 27, 2021 02:56

olavloite mentioned this pull request Apr 28, 2021

feat: support JSON data type googleapis/java-spanner-jdbc#447

Merged

feat: add JSON type support, with integration tests

f6aaa5f

zoercai force-pushed the spanner-json branch from f2dc795 to f6aaa5f Compare July 1, 2021 09:45

zoercai added 3 commits July 5, 2021 19:31

Add exception for JSON breaking change & default methods

b090697

lint

7531b71

fix

d4c2119

thiagotnunes reviewed Aug 2, 2021

View reviewed changes

review changes

288ff3e

zoercai requested a review from thiagotnunes August 17, 2021 01:17

thiagotnunes reviewed Aug 17, 2021

View reviewed changes

review changes

3fe3112

zoercai requested a review from thiagotnunes August 18, 2021 11:31

thiagotnunes approved these changes Aug 19, 2021

View reviewed changes

olavloite approved these changes Aug 19, 2021

View reviewed changes

zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 23, 2021

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 23, 2021

zoercai added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Aug 23, 2021

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 24, 2021

zoercai added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 24, 2021

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 24, 2021

gcf-merge-on-green bot merged commit d7ff940 into googleapis:master Aug 24, 2021

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Aug 24, 2021

elefeint mentioned this pull request Aug 24, 2021

Add support for JSON Spanner data type in Spring Data GoogleCloudPlatform/spring-cloud-gcp#593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for JSON data type #872

feat: add support for JSON data type #872

zoercai commented Feb 16, 2021 •

edited

Loading

generated-files-bot bot commented Feb 16, 2021

codecov bot commented Feb 16, 2021 •

edited

Loading

skuruppu commented Feb 17, 2021

thiagotnunes commented Feb 17, 2021

zoercai commented Apr 12, 2021

olavloite left a comment

thiagotnunes left a comment

zoercai commented Apr 27, 2021 •

edited

Loading

olavloite commented Apr 27, 2021

thiagotnunes commented Apr 27, 2021

thiagotnunes left a comment

thiagotnunes left a comment

feat: add support for JSON data type #872

feat: add support for JSON data type #872

Conversation

zoercai commented Feb 16, 2021 • edited Loading

generated-files-bot bot commented Feb 16, 2021

codecov bot commented Feb 16, 2021 • edited Loading

Codecov Report

skuruppu commented Feb 17, 2021

thiagotnunes commented Feb 17, 2021

zoercai commented Apr 12, 2021

olavloite left a comment

Choose a reason for hiding this comment

thiagotnunes left a comment

Choose a reason for hiding this comment

zoercai commented Apr 27, 2021 • edited Loading

olavloite commented Apr 27, 2021

thiagotnunes commented Apr 27, 2021

thiagotnunes left a comment

Choose a reason for hiding this comment

thiagotnunes left a comment

Choose a reason for hiding this comment

zoercai commented Feb 16, 2021 •

edited

Loading

codecov bot commented Feb 16, 2021 •

edited

Loading

zoercai commented Apr 27, 2021 •

edited

Loading