Skip to content

Commit

Permalink
fix: improve numeric range checks (#424)
Browse files Browse the repository at this point in the history 
* fix: improve numeric range checks

* fix: skip numeric ITs on emulator
  • Loading branch information
@olavloite
olavloite committed Sep 18, 2020
1 parent 0093f7a commit 9f26785
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 6 deletions.
28 changes: 27 additions & 1 deletion google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java
View file
Expand Up @@ -123,11 +123,37 @@ public static Value float64(double v) {
}

/**
* Returns a {@code NUMERIC} value.
* Returns a {@code NUMERIC} value. The valid value range for the whole component of the {@link
* BigDecimal} is from -9,999,999,999,999,999,999,999,999 to +9,999,999,999,999,999,999,999,999
* (both inclusive), i.e. the max length of the whole component is 29 digits. The max length of
* the fractional part is 9 digits. Trailing zeros in the fractional part are not considered and
* will be lost, as Cloud Spanner does not preserve the precision of a numeric value.
*
* <p>If you set a numeric value of a record to for example 0.10, Cloud Spanner will return this
* value as 0.1 in subsequent queries. Use {@link BigDecimal#stripTrailingZeros()} to compare
* inserted values with retrieved values if your application might insert numeric values with
* trailing zeros.
*
* @param v the value, which may be null
*/
public static Value numeric(@Nullable BigDecimal v) {
if (v != null) {
// Cloud Spanner does not preserve the precision, so 0.1 is considered equal to 0.10.
BigDecimal test = v.stripTrailingZeros();
if (test.scale() > 9) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.OUT_OF_RANGE,
String.format(
"Max scale for a numeric is 9. The requested numeric has scale %d", test.scale()));
}
if (test.precision() - test.scale() > 29) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.OUT_OF_RANGE,
String.format(
"Max precision for the whole component of a numeric is 29. The requested numeric has a whole component with precision %d",
test.precision() - test.scale()));
}
}
return new NumericImpl(v == null, v);
}

Expand Down
18 changes: 14 additions & 4 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/GrpcResultSetTest.java
View file
Expand Up @@ -25,6 +25,7 @@
import com.google.cloud.Timestamp;
import com.google.cloud.spanner.spi.v1.SpannerRpc;
import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -692,17 +693,26 @@ public void getBigDecimal() {
consumer.onPartialResultSet(
PartialResultSet.newBuilder()
.setMetadata(makeMetadata(Type.struct(Type.StructField.of("f", Type.numeric()))))
.addValues(Value.numeric(BigDecimal.valueOf(Double.MIN_VALUE)).toProto())
.addValues(Value.numeric(BigDecimal.valueOf(Double.MAX_VALUE)).toProto())
.addValues(
Value.numeric(
new BigDecimal(
"-" + Strings.repeat("9", 29) + "." + Strings.repeat("9", 9)))
.toProto())
.addValues(
Value.numeric(
new BigDecimal(Strings.repeat("9", 29) + "." + Strings.repeat("9", 9)))
.toProto())
.addValues(Value.numeric(BigDecimal.ZERO).toProto())
.addValues(Value.numeric(new BigDecimal("1.23456")).toProto())
.build());
consumer.onCompleted();

assertThat(resultSet.next()).isTrue();
assertThat(resultSet.getBigDecimal(0).doubleValue()).isWithin(0.0).of(Double.MIN_VALUE);
assertThat(resultSet.getBigDecimal(0).toPlainString())
.isEqualTo("-99999999999999999999999999999.999999999");
assertThat(resultSet.next()).isTrue();
assertThat(resultSet.getBigDecimal(0).doubleValue()).isWithin(0.0).of(Double.MAX_VALUE);
assertThat(resultSet.getBigDecimal(0).toPlainString())
.isEqualTo("99999999999999999999999999999.999999999");
assertThat(resultSet.next()).isTrue();
assertThat(resultSet.getBigDecimal(0)).isEqualTo(BigDecimal.ZERO);
assertThat(resultSet.next()).isTrue();
Expand Down
81 changes: 81 additions & 0 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/ValueTest.java
View file
Expand Up @@ -25,6 +25,7 @@
import com.google.cloud.Date;
import com.google.cloud.Timestamp;
import com.google.cloud.spanner.Type.StructField;
import com.google.common.base.Strings;
import com.google.common.collect.ForwardingList;
import com.google.common.collect.Lists;
import com.google.common.testing.EqualsTester;
Expand Down Expand Up @@ -265,6 +266,86 @@ public void testNumericFormats() {
assertThat(new BigDecimal("1e-01").toString()).isEqualTo("0.1");
}

@Test
public void numericPrecisionAndScale() {
for (long s : new long[] {1L, -1L}) {
BigDecimal sign = new BigDecimal(s);
assertThat(Value.numeric(new BigDecimal(Strings.repeat("9", 29)).multiply(sign)).toString())
.isEqualTo((s == -1L ? "-" : "") + Strings.repeat("9", 29));
try {
Value.numeric(new BigDecimal(Strings.repeat("9", 30)).multiply(sign));
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.OUT_OF_RANGE);
}
try {
Value.numeric(new BigDecimal("1" + Strings.repeat("0", 29)).multiply(sign));
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.OUT_OF_RANGE);
}

assertThat(
Value.numeric(new BigDecimal("0." + Strings.repeat("9", 9)).multiply(sign))
.toString())
.isEqualTo((s == -1L ? "-" : "") + "0." + Strings.repeat("9", 9));
assertThat(
Value.numeric(new BigDecimal("0.1" + Strings.repeat("0", 8)).multiply(sign))
.toString())
.isEqualTo((s == -1L ? "-" : "") + "0.1" + Strings.repeat("0", 8));
// Cloud Spanner does not store precision and considers 0.1 to be equal to 0.10.
// 0.100000000000000000000000000 is therefore also a valid value, as it will be capped to 0.1.
assertThat(
Value.numeric(new BigDecimal("0.1" + Strings.repeat("0", 20)).multiply(sign))
.toString())
.isEqualTo((s == -1L ? "-" : "") + "0.1" + Strings.repeat("0", 20));
try {
Value.numeric(new BigDecimal("0." + Strings.repeat("9", 10)).multiply(sign));
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.OUT_OF_RANGE);
}

assertThat(
Value.numeric(
new BigDecimal(Strings.repeat("9", 29) + "." + Strings.repeat("9", 9))
.multiply(sign))
.toString())
.isEqualTo(
(s == -1L ? "-" : "") + Strings.repeat("9", 29) + "." + Strings.repeat("9", 9));

try {
Value.numeric(
new BigDecimal(Strings.repeat("9", 30) + "." + Strings.repeat("9", 9)).multiply(sign));
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.OUT_OF_RANGE);
}
try {
Value.numeric(
new BigDecimal("1" + Strings.repeat("0", 29) + "." + Strings.repeat("9", 9))
.multiply(sign));
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.OUT_OF_RANGE);
}

try {
Value.numeric(
new BigDecimal(Strings.repeat("9", 29) + "." + Strings.repeat("9", 10)).multiply(sign));
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.OUT_OF_RANGE);
}
try {
Value.numeric(new BigDecimal("1." + Strings.repeat("9", 10)).multiply(sign));
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getErrorCode()).isEqualTo(ErrorCode.OUT_OF_RANGE);
}
}
}

@Test
public void numericNull() {
Value v = Value.numeric(null);
Expand Down
83 changes: 82 additions & 1 deletion google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITQueryTest.java
View file
Expand Up @@ -16,7 +16,6 @@

package com.google.cloud.spanner.it;

import static com.google.cloud.spanner.Type.StructField;
import static com.google.cloud.spanner.testing.EmulatorSpannerHelper.isUsingEmulator;
import static com.google.common.truth.Truth.assertThat;
import static java.util.Arrays.asList;
Expand All @@ -39,10 +38,13 @@
import com.google.cloud.spanner.Struct;
import com.google.cloud.spanner.TimestampBound;
import com.google.cloud.spanner.Type;
import com.google.cloud.spanner.Type.StructField;
import com.google.cloud.spanner.Value;
import com.google.cloud.spanner.testing.EmulatorSpannerHelper;
import com.google.common.base.Joiner;
import com.google.common.collect.Iterables;
import com.google.spanner.v1.ResultSetStats;
import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -269,6 +271,34 @@ public void bindDateNull() {
assertThat(row.isNull(0)).isTrue();
}

@Test
public void bindNumeric() {
assumeFalse("Emulator does not yet support NUMERIC", EmulatorSpannerHelper.isUsingEmulator());
BigDecimal b = new BigDecimal("1.1");
Struct row = execute(Statement.newBuilder("SELECT @v").bind("v").to(b), Type.numeric());
assertThat(row.isNull(0)).isFalse();
assertThat(row.getBigDecimal(0)).isEqualTo(b);
}

@Test
public void bindNumericNull() {
assumeFalse("Emulator does not yet support NUMERIC", EmulatorSpannerHelper.isUsingEmulator());
Struct row =
execute(Statement.newBuilder("SELECT @v").bind("v").to((BigDecimal) null), Type.numeric());
assertThat(row.isNull(0)).isTrue();
}

@Test
public void bindNumeric_doesNotPreservePrecision() {
assumeFalse("Emulator does not yet support NUMERIC", EmulatorSpannerHelper.isUsingEmulator());
BigDecimal b = new BigDecimal("1.10");
Struct row = execute(Statement.newBuilder("SELECT @v").bind("v").to(b), Type.numeric());
assertThat(row.isNull(0)).isFalse();
// Cloud Spanner does not store precision, and will therefore return 1.10 as 1.1.
assertThat(row.getBigDecimal(0)).isNotEqualTo(b);
assertThat(row.getBigDecimal(0)).isEqualTo(b.stripTrailingZeros());
}

@Test
public void bindBoolArray() {
Struct row =
Expand Down Expand Up @@ -494,6 +524,57 @@ public void bindDateArrayNull() {
assertThat(row.isNull(0)).isTrue();
}

@Test
public void bindNumericArray() {
assumeFalse("Emulator does not yet support NUMERIC", EmulatorSpannerHelper.isUsingEmulator());
BigDecimal b1 = new BigDecimal("3.14");
BigDecimal b2 = new BigDecimal("6.626");

Struct row =
execute(
Statement.newBuilder("SELECT @v").bind("v").toNumericArray(asList(b1, b2, null)),
Type.array(Type.numeric()));
assertThat(row.isNull(0)).isFalse();
assertThat(row.getBigDecimalList(0)).containsExactly(b1, b2, null).inOrder();
}

@Test
public void bindNumericArrayEmpty() {
assumeFalse("Emulator does not yet support NUMERIC", EmulatorSpannerHelper.isUsingEmulator());
Struct row =
execute(
Statement.newBuilder("SELECT @v").bind("v").toNumericArray(Arrays.<BigDecimal>asList()),
Type.array(Type.numeric()));
assertThat(row.isNull(0)).isFalse();
assertThat(row.getBigDecimalList(0)).containsExactly();
}

@Test
public void bindNumericArrayNull() {
assumeFalse("Emulator does not yet support NUMERIC", EmulatorSpannerHelper.isUsingEmulator());
Struct row =
execute(
Statement.newBuilder("SELECT @v").bind("v").toNumericArray(null),
Type.array(Type.numeric()));
assertThat(row.isNull(0)).isTrue();
}

@Test
public void bindNumericArray_doesNotPreservePrecision() {
assumeFalse("Emulator does not yet support NUMERIC", EmulatorSpannerHelper.isUsingEmulator());
BigDecimal b1 = new BigDecimal("3.14");
BigDecimal b2 = new BigDecimal("6.626070");

Struct row =
execute(
Statement.newBuilder("SELECT @v").bind("v").toNumericArray(asList(b1, b2, null)),
Type.array(Type.numeric()));
assertThat(row.isNull(0)).isFalse();
assertThat(row.getBigDecimalList(0))
.containsExactly(b1.stripTrailingZeros(), b2.stripTrailingZeros(), null)
.inOrder();
}

@Test
public void unsupportedSelectStructValue() {
assumeFalse("The emulator accepts this query", isUsingEmulator());
Expand Down

0 comments on commit 9f26785

Please sign in to comment.