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

feat: add support for JSON data type #872

Merged
merged 6 commits into from Aug 24, 2021

Conversation

zoercai
Copy link
Contributor

@zoercai zoercai commented Feb 16, 2021

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

Integration tests here: zoercai#1

@zoercai 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 zoercai requested review from a team as code owners February 16, 2021 10:02
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 16, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Feb 16, 2021
@generated-files-bot
Copy link

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
Copy link

codecov bot commented Feb 16, 2021

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
Impacted file tree graph

@@             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
Copy link
Contributor

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

@thiagotnunes
Copy link
Contributor

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

@zoercai
Copy link
Contributor Author

zoercai commented Apr 12, 2021

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?

Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@thiagotnunes thiagotnunes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@zoercai
Copy link
Contributor Author

zoercai commented Apr 27, 2021

@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
Copy link
Collaborator

@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
Copy link
Contributor

@zoercai also added comments on the other PR

Copy link
Contributor

@thiagotnunes thiagotnunes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@thiagotnunes thiagotnunes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zoercai zoercai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 23, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 23, 2021
@zoercai 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 yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 24, 2021
@zoercai 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 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 gcf-merge-on-green bot merged commit d7ff940 into googleapis:master Aug 24, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Aug 24, 2021
sgammon pushed a commit to sgammon/elide-archive that referenced this pull request Mar 16, 2022
* 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 <bot@renovateapp.com>
ansh0l pushed a commit to ansh0l/java-spanner that referenced this pull request Nov 10, 2022
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
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
…3.0 (googleapis#872)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.cloud:libraries-bom](https://cloud.google.com/java/docs/bom) ([source](https://togithub.com/GoogleCloudPlatform/cloud-opensource-java)) | `25.2.0` -> `25.3.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/25.3.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/25.3.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/25.3.0/compatibility-slim/25.2.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/25.3.0/confidence-slim/25.2.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-spanner-jdbc).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants