From 6f029b4f6dc0c7475ddc2da364b0c7fd7c93726d Mon Sep 17 00:00:00 2001 From: gauravpurohit Date: Thu, 3 Nov 2022 09:07:00 +0000 Subject: [PATCH 1/3] refactor: Removing duplicated code to handle array values The same deserialization logic for data types have been duplicated at multiple places which is prone to error. --- .../cloud/spanner/AbstractResultSet.java | 72 ++----------------- 1 file changed, 4 insertions(+), 68 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java index 6ccb28900f9..272cd2de308 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java @@ -34,7 +34,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.AbstractIterator; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; import com.google.common.util.concurrent.Uninterruptibles; import com.google.protobuf.ByteString; import com.google.protobuf.ListValue; @@ -540,88 +539,25 @@ private static Struct decodeStructValue(Type structType, ListValue structValue) static Object decodeArrayValue(Type elementType, ListValue listValue) { switch (elementType.getCode()) { - case BOOL: - // Use a view: element conversion is virtually free. - return Lists.transform( - listValue.getValuesList(), - input -> input.getKindCase() == KindCase.NULL_VALUE ? null : input.getBoolValue()); case INT64: // For int64/float64 types, use custom containers. These avoid wrapper object // creation for non-null arrays. return new Int64Array(listValue); case FLOAT64: return new Float64Array(listValue); + case BOOL: case NUMERIC: - { - // Materialize list: element conversion is expensive and should happen only once. - ArrayList list = new ArrayList<>(listValue.getValuesCount()); - for (com.google.protobuf.Value value : listValue.getValuesList()) { - list.add( - value.getKindCase() == KindCase.NULL_VALUE - ? null - : new BigDecimal(value.getStringValue())); - } - return list; - } case PG_NUMERIC: case STRING: case JSON: case PG_JSONB: - return listValue.getValuesList().stream() - .map( - input -> - input.getKindCase() == KindCase.NULL_VALUE ? null : input.getStringValue()) - .collect(Collectors.toList()); case BYTES: - { - // Materialize list: element conversion is expensive and should happen only once. - ArrayList list = new ArrayList<>(listValue.getValuesCount()); - for (com.google.protobuf.Value value : listValue.getValuesList()) { - list.add( - value.getKindCase() == KindCase.NULL_VALUE - ? null - : ByteArray.fromBase64(value.getStringValue())); - } - return list; - } case TIMESTAMP: - { - // Materialize list: element conversion is expensive and should happen only once. - ArrayList list = new ArrayList<>(listValue.getValuesCount()); - for (com.google.protobuf.Value value : listValue.getValuesList()) { - list.add( - value.getKindCase() == KindCase.NULL_VALUE - ? null - : Timestamp.parseTimestamp(value.getStringValue())); - } - return list; - } case DATE: - { - // Materialize list: element conversion is expensive and should happen only once. - ArrayList list = new ArrayList<>(listValue.getValuesCount()); - for (com.google.protobuf.Value value : listValue.getValuesList()) { - list.add( - value.getKindCase() == KindCase.NULL_VALUE - ? null - : Date.parseDate(value.getStringValue())); - } - return list; - } - case STRUCT: - { - ArrayList list = new ArrayList<>(listValue.getValuesCount()); - for (com.google.protobuf.Value value : listValue.getValuesList()) { - if (value.getKindCase() == KindCase.NULL_VALUE) { - list.add(null); - } else { - ListValue structValue = value.getListValue(); - list.add(decodeStructValue(elementType, structValue)); - } - } - return list; - } + return listValue.getValuesList().stream() + .map(input -> decodeValue(elementType, input)) + .collect(Collectors.toList()); default: throw new AssertionError("Unhandled type code: " + elementType.getCode()); } From ea0db211e23c13381d1bb044f9ac5d999049ec93 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Thu, 3 Nov 2022 09:21:06 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e78b8adb756..df8bf4d1ae5 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ If you are using Maven without BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.1.3') +implementation platform('com.google.cloud:libraries-bom:26.1.4') implementation 'com.google.cloud:google-cloud-spanner' ``` From 33d25fcdcc984caa60b991ee83ae2507c7a9b08e Mon Sep 17 00:00:00 2001 From: gauravpurohit Date: Wed, 7 Dec 2022 05:29:08 +0000 Subject: [PATCH 3/3] refactor: Using lazy computation for all the data types and adding docs --- .../cloud/spanner/AbstractResultSet.java | 7 +- .../google/cloud/spanner/StructReader.java | 112 ++++++++++++++---- 2 files changed, 93 insertions(+), 26 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java index 98cf9ccb328..789e0945e17 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java @@ -34,6 +34,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.AbstractIterator; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; import com.google.common.util.concurrent.Uninterruptibles; import com.google.protobuf.ByteString; import com.google.protobuf.ListValue; @@ -66,7 +67,6 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; import javax.annotation.Nullable; /** Implementation of {@link ResultSet}. */ @@ -562,9 +562,8 @@ static Object decodeArrayValue(Type elementType, ListValue listValue) { case TIMESTAMP: case DATE: case STRUCT: - return listValue.getValuesList().stream() - .map(input -> decodeValue(elementType, input)) - .collect(Collectors.toList()); + return Lists.transform( + listValue.getValuesList(), input -> decodeValue(elementType, input)); default: throw new AssertionError("Unhandled type code: " + elementType.getCode()); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/StructReader.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/StructReader.java index a96c95cb953..b767bd6d82c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/StructReader.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/StructReader.java @@ -178,10 +178,18 @@ default Value getValue(String columnName) { */ boolean[] getBooleanArray(String columnName); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bool())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bool())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getBooleanList(int columnIndex); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bool())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bool())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getBooleanList(String columnName); /** @@ -200,10 +208,18 @@ default Value getValue(String columnName) { */ long[] getLongArray(String columnName); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.int64())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.int64())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getLongList(int columnIndex); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.int64())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.int64())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getLongList(String columnName); /** @@ -223,84 +239,136 @@ default Value getValue(String columnName) { double[] getDoubleArray(String columnName); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.float64())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.float64())} The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. */ List getDoubleList(int columnIndex); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.float64())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.float64())} The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. */ List getDoubleList(String columnName); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.numeric())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.numeric())} The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. */ List getBigDecimalList(int columnIndex); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.numeric())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.numeric())} The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. */ List getBigDecimalList(String columnName); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.string())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.string())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getStringList(int columnIndex); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.string())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.string())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getStringList(String columnName); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.json())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.json())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ default List getJsonList(int columnIndex) { throw new UnsupportedOperationException("method should be overwritten"); }; - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.json())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.json())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ default List getJsonList(String columnName) { throw new UnsupportedOperationException("method should be overwritten"); }; /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.pgJsonb())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.pgJsonb())} The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. */ default List getPgJsonbList(int columnIndex) { throw new UnsupportedOperationException("method should be overwritten"); }; /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.pgJsonb())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.pgJsonb())} The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. */ default List getPgJsonbList(String columnName) { throw new UnsupportedOperationException("method should be overwritten"); }; - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bytes())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bytes())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getBytesList(int columnIndex); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bytes())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.bytes())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getBytesList(String columnName); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.timestamp())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.timestamp())} + * The list returned by this method is lazily constructed. Create a copy of it if you intend to + * access each element in the list multiple times. */ List getTimestampList(int columnIndex); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.timestamp())}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.timestamp())} + * The list returned by this method is lazily constructed. Create a copy of it if you intend to + * access each element in the list multiple times. */ List getTimestampList(String columnName); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.date())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.date())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getDateList(int columnIndex); - /** Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.date())}. */ + /** + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.date())}. The + * list returned by this method is lazily constructed. Create a copy of it if you intend to access + * each element in the list multiple times. + */ List getDateList(String columnName); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.struct(...))}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.struct(...))} + * The list returned by this method is lazily constructed. Create a copy of it if you intend to + * access each element in the list multiple times. */ List getStructList(int columnIndex); /** - * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.struct(...))}. + * Returns the value of a non-{@code NULL} column with type {@code Type.array(Type.struct(...))} + * The list returned by this method is lazily constructed. Create a copy of it if you intend to + * access each element in the list multiple times. */ List getStructList(String columnName); }