From 182906f7a952991cf36c8401465c883fcc0f63e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 28 Apr 2022 16:13:05 +0200 Subject: [PATCH] chore: support untyped values for statement parameters (#1854) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../cloud/spanner/AbstractReadContext.java | 4 +- .../google/cloud/spanner/BatchClientImpl.java | 2 +- .../spanner/PartitionedDmlTransaction.java | 2 +- .../java/com/google/cloud/spanner/Value.java | 65 ++++++++++++++++++- .../google/cloud/spanner/ValueBinderTest.java | 7 +- .../com/google/cloud/spanner/ValueTest.java | 30 +++++++++ 6 files changed, 103 insertions(+), 7 deletions(-) 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 index ea658a36eab..60324f8e83e 100644 --- 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 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 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 index 222d754dac9..5a164d52490 100644 --- 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 partitionQuery( Struct.Builder paramsBuilder = builder.getParamsBuilder(); for (Map.Entry 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 index 78c092f1792..cbe01c2d33d 100644 --- 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 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 index 0bbeb36abf0..e3c53de9377 100644 --- 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 index 122f5582736..ea26f09c2ed 100644 --- 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 index 08849274de5..54f6799e8c8 100644 --- 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 Iterable 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);