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.

Sign up for GitHub

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

Jump to bottom

fix: improve numeric range checks #424

Merged
merged 2 commits into from Sep 18, 2020
Merged

fix: improve numeric range checks #424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e00545
fix: improve numeric range checks
olavloite Sep 14, 2020
abb579c
fix: skip numeric ITs on emulator
olavloite Sep 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 27 additions & 1 deletion google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java
View file
Open in desktop
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.
olavloite marked this conversation as resolved.
Show resolved Hide resolved
*
* @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
Open in desktop
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
Open in desktop
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
Open in desktop
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