chore: support untyped values for statement parameters (#1854)

olavloite · gcf-owl-bot[bot] · web-flow · commit 182906f7a952 · 2022-04-28T16:13:05.000+02:00
* feat: support untyped values * chore: support untyped values Adds support for untyped values that can be used as statement parameters. This is required for Spangres, as many native PG drivers send (some) parameters without an explicit type, and expect the backend to infer the type from the context. If we were to include a (placeholder) type for such a parameter, the type inference on the backend fails. * chore: support untyped values Adds support for untyped values that can be used as statement parameters. This is required for Spangres, as many native PG drivers send (some) parameters without an explicit type, and expect the backend to infer the type from the context. If we were to include a (placeholder) type for such a parameter, the type inference on the backend fails. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java
@@ -584,7 +584,7 @@ ExecuteSqlRequest.Builder getExecuteSqlRequestBuilder(
       com.google.protobuf.Struct.Builder paramsBuilder = builder.getParamsBuilder();
       for (Map.Entry<String, Value> param : stmtParameters.entrySet()) {
         paramsBuilder.putFields(param.getKey(), Value.toProto(param.getValue()));
-        if (param.getValue() != null) {
+        if (param.getValue() != null && param.getValue().getType() != null) {
           builder.putParamTypes(param.getKey(), param.getValue().getType().toProto());
         }
       }
@@ -615,7 +615,7 @@ ExecuteBatchDmlRequest.Builder getExecuteBatchDmlRequestBuilder(
             builder.getStatementsBuilder(idx).getParamsBuilder();
         for (Map.Entry<String, Value> param : stmtParameters.entrySet()) {
           paramsBuilder.putFields(param.getKey(), Value.toProto(param.getValue()));
-          if (param.getValue() != null) {
+          if (param.getValue() != null && param.getValue().getType() != null) {
             builder
                 .getStatementsBuilder(idx)
                 .putParamTypes(param.getKey(), param.getValue().getType().toProto());
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java
@@ -170,7 +170,7 @@ public List<Partition> partitionQuery(
         Struct.Builder paramsBuilder = builder.getParamsBuilder();
         for (Map.Entry<String, Value> param : stmtParameters.entrySet()) {
           paramsBuilder.putFields(param.getKey(), Value.toProto(param.getValue()));
-          if (param.getValue() != null) {
+          if (param.getValue() != null && param.getValue().getType() != null) {
             builder.putParamTypes(param.getKey(), param.getValue().getType().toProto());
           }
         }
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDmlTransaction.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDmlTransaction.java
@@ -218,7 +218,7 @@ private void setParameters(
       com.google.protobuf.Struct.Builder paramsBuilder = requestBuilder.getParamsBuilder();
       for (Map.Entry<String, Value> param : statementParameters.entrySet()) {
         paramsBuilder.putFields(param.getKey(), Value.toProto(param.getValue()));
-        if (param.getValue() != null) {
+        if (param.getValue() != null && param.getValue().getType() != null) {
           requestBuilder.putParamTypes(param.getKey(), param.getValue().getType().toProto());
         }
       }
diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java
@@ -87,6 +87,17 @@ public abstract class Value implements Serializable {
   private static final char LIST_CLOSE = ']';
   private static final long serialVersionUID = -5289864325087675338L;
 
+  /**
+   * Returns a {@link Value} that wraps the given proto value. This can be used to construct a value
+   * without a specific type, and let the backend infer the type based on the statement where it is
+   * used.
+   *
+   * @param value the non-null proto value (a {@link NullValue} is allowed)
+   */
+  public static Value untyped(com.google.protobuf.Value value) {
+    return new UntypedValueImpl(Preconditions.checkNotNull(value));
+  }
+
   /**
    * Returns a {@code BOOL} value.
    *
@@ -914,7 +925,7 @@ public final boolean equals(Object o) {
       }
 
       AbstractValue that = (AbstractValue) o;
-      if (!getType().equals(that.getType()) || isNull != that.isNull) {
+      if (!Objects.equals(getType(), that.getType()) || isNull != that.isNull) {
         return false;
       }
 
@@ -963,6 +974,58 @@ final void checkNotNull() {
     }
   }
 
+  private static class UntypedValueImpl extends AbstractValue {
+    private final com.google.protobuf.Value value;
+
+    private UntypedValueImpl(com.google.protobuf.Value value) {
+      super(value.hasNullValue(), null);
+      this.value = value;
+    }
+
+    @Override
+    public boolean getBool() {
+      checkNotNull();
+      Preconditions.checkState(value.hasBoolValue(), "This value does not contain a bool value");
+      return value.getBoolValue();
+    }
+
+    @Override
+    public String getString() {
+      checkNotNull();
+      Preconditions.checkState(
+          value.hasStringValue(), "This value does not contain a string value");
+      return value.getStringValue();
+    }
+
+    @Override
+    public double getFloat64() {
+      checkNotNull();
+      Preconditions.checkState(
+          value.hasNumberValue(), "This value does not contain a number value");
+      return value.getNumberValue();
+    }
+
+    @Override
+    void valueToString(StringBuilder b) {
+      b.append(value);
+    }
+
+    @Override
+    com.google.protobuf.Value valueToProto() {
+      return value;
+    }
+
+    @Override
+    boolean valueEquals(Value v) {
+      return ((UntypedValueImpl) v).value.equals(value);
+    }
+
+    @Override
+    int valueHash() {
+      return value.hashCode();
+    }
+  }
+
   private static class BoolImpl extends AbstractValue {
     private final boolean value;
 
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueBinderTest.java
@@ -56,10 +56,13 @@ Integer handle(Value value) {
   public void reflection()
       throws InvocationTargetException, IllegalAccessException, NoSuchMethodException {
     // Test that every Value factory method has a counterpart in ValueBinder, and that invoking it
-    // produces the expected Value.
+    // produces the expected Value. The only exception is for untyped values, which must be
+    // constructed manually as an untyped value and then assigned as a parameter.
     for (Method method : Value.class.getMethods()) {
       if (!Modifier.isStatic(method.getModifiers())
-          || !method.getReturnType().equals(Value.class)) {
+          || !method.getReturnType().equals(Value.class)
+          || (method.getParameterTypes().length > 0
+              && method.getParameterTypes()[0].equals(com.google.protobuf.Value.class))) {
         continue;
       }
       Method binderMethod = findBinderMethod(method);
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueTest.java
@@ -22,7 +22,9 @@
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -63,6 +65,34 @@ private static <T> Iterable<T> plainIterable(T... values) {
     return Lists.newArrayList(values);
   }
 
+  @Test
+  public void untyped() {
+    com.google.protobuf.Value proto =
+        com.google.protobuf.Value.newBuilder().setStringValue("test").build();
+    Value v = Value.untyped(proto);
+    assertNull(v.getType());
+    assertFalse(v.isNull());
+    assertSame(proto, v.toProto());
+
+    assertEquals(
+        v, Value.untyped(com.google.protobuf.Value.newBuilder().setStringValue("test").build()));
+    assertEquals(
+        Value.untyped(com.google.protobuf.Value.newBuilder().setNumberValue(3.14d).build()),
+        Value.untyped(com.google.protobuf.Value.newBuilder().setNumberValue(3.14d).build()));
+    assertEquals(
+        Value.untyped(com.google.protobuf.Value.newBuilder().setBoolValue(true).build()),
+        Value.untyped(com.google.protobuf.Value.newBuilder().setBoolValue(true).build()));
+
+    assertNotEquals(
+        v, Value.untyped(com.google.protobuf.Value.newBuilder().setStringValue("foo").build()));
+    assertNotEquals(
+        Value.untyped(com.google.protobuf.Value.newBuilder().setNumberValue(3.14d).build()),
+        Value.untyped(com.google.protobuf.Value.newBuilder().setNumberValue(0.14d).build()));
+    assertNotEquals(
+        Value.untyped(com.google.protobuf.Value.newBuilder().setBoolValue(false).build()),
+        Value.untyped(com.google.protobuf.Value.newBuilder().setBoolValue(true).build()));
+  }
+
   @Test
   public void bool() {
     Value v = Value.bool(true);

Original file line number	Diff line number	Diff line change
`@@ -170,7 +170,7 @@ public List<Partition> partitionQuery(`
`170`	`170`	`Struct.Builder paramsBuilder = builder.getParamsBuilder();`
`171`	`171`	`for (Map.Entry<String, Value> param : stmtParameters.entrySet()) {`
`172`	`172`	`paramsBuilder.putFields(param.getKey(), Value.toProto(param.getValue()));`
`173`		`- if (param.getValue() != null) {`
	`173`	`+ if (param.getValue() != null && param.getValue().getType() != null) {`
`174`	`174`	`builder.putParamTypes(param.getKey(), param.getValue().getType().toProto());`
`175`	`175`	`}`
`176`	`176`	`}`
Original file line number	Diff line number	Diff line change
`@@ -218,7 +218,7 @@ private void setParameters(`
`218`	`218`	`com.google.protobuf.Struct.Builder paramsBuilder = requestBuilder.getParamsBuilder();`
`219`	`219`	`for (Map.Entry<String, Value> param : statementParameters.entrySet()) {`
`220`	`220`	`paramsBuilder.putFields(param.getKey(), Value.toProto(param.getValue()));`
`221`		`- if (param.getValue() != null) {`
	`221`	`+ if (param.getValue() != null && param.getValue().getType() != null) {`
`222`	`222`	`requestBuilder.putParamTypes(param.getKey(), param.getValue().getType().toProto());`
`223`	`223`	`}`
`224`	`224`	`}`