feat: support UNRECOGNIZED types + decode BYTES columns lazily #2219

olavloite · 2023-01-05T16:24:25Z

Unrecognized Types

Adds support for types that do not have a known type code in the client library (yet). This can happen if a new type is added to Cloud Spanner before this client is updated, or if someone is using an old version of the client library with a version of Cloud Spanner that contains a type that was not available at the time that the specific version of the client library was built.

Unrecognized types are assigned a type with type code UNRECOGNIZED. The values of these types (and all other types) can be retrieved as follows:

Calling ResultSet#getValue(..) will return the column value as a Value instance.
Calling Value#getAsString() will return a string representation of the given value. The string value will be a valid and complete representation of the underlying value. This method is guaranteed to work for all known and unknown types. Array types will return a comma-separated string enclosed in square brackets (e.g. [true, NULL, false] for a boolean array).
Calling Value#getAsStringList() will return a list of strings. The list of strings will contain:
1. In case the value is an array: One string for each element in the array. Each element is a complete and valid string representation of the underlying array element value.
2. In case the value is a non-array type: A singleton list containing only the string representation of the specific value.
3. A user can always call Value#getType()#getCode() == Code.ARRAY to check whether a value is an array with one element, or whether it is a non-array type to determine what the reason is that getAsStringList() returns a list with one element.

Decoding BYTES

BYTES columns are encoded as Base64 strings. Decoding these are relatively CPU-heavy, especially for large values. Decoding them is not always necessary if the user only needs the Base64 string. Also, the internally used Guava decoder is less efficient than JDK implementations that are available from Java 8 and onwards. This change therefore delays the decoding of BYTES columns until it is actually necessary, and then uses the JDK implementation instead of the Guava version. The JDK implementation in OpenJDK 17 uses approx 1/3 of the CPU cycles of the Guava version.

This feature is combined with unrecognized types so it can make use of the new getAsString() methods. This prevents the introduction of what is (theoretically) a breaking change. Currently, Value#getString(..) throws an exception if you try to call this on a BYTES value. This remains the case after this change, and instead users should call Value#getAsString() to get the underlying Base64 value of a BYTES column.

BYTES columns are encoded as Base64 strings. Decoding these are relatively CPU-heavy, especially for large values. Decoding them is not always necessary if the user only needs the Base64 string. Also, the internally used Guava decoder is less efficient than JDK implementations that are available from Java 8 and onwards. This change therefore delays the decoding of BYTES columns until it is actually necessary, and then uses the JDK implementation instead of the Guava version. The JDK implementation in OpenJDK 17 uses approx 1/3 of the CPU cycles of the Guava version.

…unrecognized

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

aseering

Just did a quick scan and left a comment or two.

aseering · 2023-01-20T14:13:59Z

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

+        b.append("ARRAY<");
+      } else {
+        // This is very unlikely to happen. It would mean that we have introduced a type that
+        // is not an ARRAY, but does have an array element type.


Can the new PROTO type do this? @charvisingla

Proto type has its own field called proto_type_fqn for specifying the proto type name. I agree that its very unlikely to use array_element_type field for a non array type.

thiagotnunes · 2023-01-23T01:51:01Z

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

@@ -251,7 +251,6 @@ public Date getDate(String columnName) {

  @Override
  public Value getValue(int columnIndex) {
-    checkNonNull(columnIndex, columnIndex);


Is this a breaking change? We are modifying the contract here, I wonder if any customer is relying on this? I would imagine not, but double checking.

Hmm... That's a good point. I don't think anyone would be relying on it, but you're right that it is a change of the contract. In this case we will always be returning a non-null value, even if the database value is NULL (we're returning a Value instance whose internal value is null and whose type is set to the type of the column), so it also a safe value to return. I think there are two (three) options here:

Accept the small change in contract.

Add an extra method named something like 'getValueOrNull`

Do a major version bump (but I don't feel like this feature is worth that)

@thiagotnunes @rajatbhatta @ansh0l Any specific thoughts on this?

1 makes sense to me as well. As discussed offline, the documentation for getValue also indicates that this method can be used with columns having a null value.

thiagotnunes · 2023-01-23T01:52:07Z

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

@@ -204,13 +210,23 @@ private Type(
      Code code,
      @Nullable Type arrayElementType,
      @Nullable ImmutableList<StructField> structFields) {
+    this.proto = null;


Could we always populate the corresponding proto type?

That's a little difficult. This constructor is used to create the fixed type constants for all known types in the client library. They do not have a proto definition available at construction time. See for example here:

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

Line 49 in f637f1e

private static final Type TYPE_BOOL = new Type(Code.BOOL, null, null);

rajatbhatta

LGTM, apart from a few nits or questions. Thank you @olavloite for working on this!

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

rajatbhatta · 2023-01-31T08:39:01Z

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

@@ -251,7 +251,6 @@ public Date getDate(String columnName) {

  @Override
  public Value getValue(int columnIndex) {
-    checkNonNull(columnIndex, columnIndex);


1 makes sense to me as well. As discussed offline, the documentation for getValue also indicates that this method can be used with columns having a null value.

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

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

...loud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

olavloite · 2023-02-02T10:10:49Z

CI failure for Windows is a setup failure for that CI run and not related to this change.

olavloite · 2023-11-09T09:25:35Z

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

+                                        .setName("c")
+                                        .setType(
+                                            Type.newBuilder()
+                                                .setCodeValue(Integer.MAX_VALUE)


This line sets a type code value that does not exist, and will be seen as an unrecognized type in the client library

product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner API. labels Jan 5, 2023

feat: support unrecognized types

066f6dc

product-auto-label bot added size: xl Pull request size is extra large. and removed size: m Pull request size is medium. labels Jan 11, 2023

olavloite added 3 commits January 12, 2023 19:31

feat: add support for lazy bytes arrays

dd73f39

Merge branch 'main' into lazy-decode-bytes

4060866

chore: cleanup

617120d

olavloite marked this pull request as ready for review January 13, 2023 12:58

olavloite requested a review from a team as a code owner January 13, 2023 12:58

olavloite changed the title ~~perf: decode BYTES columns lazily~~ feat: support UNRECOGNIZED types + decode BYTES columns lazily Jan 13, 2023

olavloite and others added 2 commits January 13, 2023 14:25

fix: unrecognized types with array element type should be considered …

59d12eb

…unrecognized

🦉 Updates from OwlBot post-processor

f637f1e

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

gcf-owl-bot bot requested a review from a team as a code owner January 13, 2023 13:28

aseering reviewed Jan 20, 2023

View reviewed changes

thiagotnunes approved these changes Jan 23, 2023

View reviewed changes

olavloite requested a review from rajatbhatta January 23, 2023 19:27

rajatbhatta approved these changes Feb 1, 2023

View reviewed changes

olavloite and others added 3 commits February 2, 2023 10:12

chore: address review comments

e59edc7

Merge branch 'main' into lazy-decode-bytes

fd7e852

🦉 Updates from OwlBot post-processor

c849aff

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

olavloite merged commit fc721c4 into main Feb 2, 2023

olavloite deleted the lazy-decode-bytes branch February 2, 2023 10:10

release-please bot mentioned this pull request Feb 2, 2023

chore(main): release 6.36.0 #2260

Merged

olavloite mentioned this pull request Mar 29, 2023

test: create prototype for values with unrecognized types #2095

Closed

olavloite commented Nov 9, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support UNRECOGNIZED types + decode BYTES columns lazily #2219

feat: support UNRECOGNIZED types + decode BYTES columns lazily #2219

olavloite commented Jan 5, 2023 •

edited

Loading

aseering left a comment

aseering Jan 20, 2023

charvisingla Jan 31, 2023

thiagotnunes Jan 23, 2023

olavloite Jan 23, 2023

rajatbhatta Jan 31, 2023

thiagotnunes Jan 23, 2023

olavloite Jan 23, 2023

rajatbhatta left a comment

rajatbhatta Jan 31, 2023

olavloite commented Feb 2, 2023

olavloite Nov 9, 2023

feat: support UNRECOGNIZED types + decode BYTES columns lazily #2219

feat: support UNRECOGNIZED types + decode BYTES columns lazily #2219

Conversation

olavloite commented Jan 5, 2023 • edited Loading

Unrecognized Types

Decoding BYTES

aseering left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajatbhatta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olavloite commented Feb 2, 2023

Choose a reason for hiding this comment

olavloite commented Jan 5, 2023 •

edited

Loading