Skip to content

feat(spanner): add support for Proto Columns in Connection API #3123

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.

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

Merged
merged 37 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
cbabd6c
feat: Support for Proto Messages & Enums (#2155)
gauravpurohit06 Dec 28, 2022
1fb6e94
samples: Adding samples for updating & querying Proto messages & enum…
gauravpurohit06 Jan 5, 2023
6c97b99
test: Proto Column Integration tests (#2212)
gauravpurohit06 Jan 5, 2023
dd358c2
Merge remote-tracking branch 'upstream/main' into proto-column-enhanc…
gauravpurohit06 Jan 5, 2023
53397e6
Configured jitpack.yml to use OpenJDK 11 (#2218)
gauravpurohit06 Jan 5, 2023
2f8eb42
feat: add support for Proto Columns DDL (#2277)
harshachinta Mar 15, 2023
a70f259
Merge branch 'main' into proto-column-enhancement-alpha
harshachinta Jun 8, 2023
9513753
teat: update pom file to run tests on cloud-devel region temporarily …
harshachinta Jun 8, 2023
6701f52
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jun 8, 2023
f63da18
fix: revert host changes in pom.xml file
harshachinta Jun 9, 2023
10ee620
feat: add support for Proto Columns to Connection API
harshachinta Jun 9, 2023
7feaf10
feat: add client side statement support for proto descriptor
harshachinta Jun 9, 2023
02a31b6
feat: update existing unit tests to include proto descriptorss argument
harshachinta Jun 10, 2023
7746e08
feat: code refactor for client side statements
harshachinta Jun 12, 2023
dcbe166
feat: add unit tests
harshachinta Jun 12, 2023
76fdd77
feat: code refactoring for file path client statement
harshachinta Jun 12, 2023
33044c4
test: add integration test for DDL
harshachinta Jun 12, 2023
ad0873a
fix: comment refactoring
harshachinta Jun 13, 2023
a5f1697
feat: move proto descriptor file read logic to ConnectionImpl file to…
harshachinta Jun 13, 2023
e8fed61
feat: add autogenerated test for new proto columns client side statem…
harshachinta Jun 13, 2023
b15dd75
feat: add unit tests to verify proto descriptor set via filepath
harshachinta Jun 14, 2023
0c9f4f6
feat: add review comments
harshachinta Jun 15, 2023
f97b249
feat: add client side statement to show proto descriptors file path
harshachinta Jun 15, 2023
a42761b
fix: remove proto descriptors file extension validation
harshachinta Jun 15, 2023
865fc0c
feat: comment refactor
harshachinta Jun 15, 2023
f1e3f3a
feat: address review comments
harshachinta Jun 23, 2023
ef6e043
Merge branch 'main' into jdbc-proto-column-feature
harshachinta May 23, 2024
3481cfe
feat: update tests and revert autogenerated code
harshachinta May 23, 2024
8468d9a
chore: revert autogenerated coee
harshachinta May 23, 2024
5e0bed1
chore: revert autogenerated code
harshachinta May 23, 2024
0e4361f
chore: revert autogenerated code
harshachinta May 23, 2024
1f0d006
chore: lint format
harshachinta May 23, 2024
771178a
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 23, 2024
f7b5cf6
chore: regenerate descriptors file
harshachinta May 23, 2024
b98b56e
chore: update schema
harshachinta May 23, 2024
85bb7f3
chore: update base64 value for protodescriptor
harshachinta May 24, 2024
94eb11c
chore: update copyright year
harshachinta May 24, 2024
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ implementation 'com.google.cloud:google-cloud-spanner'
If you are using Gradle without BOM, add this to your dependencies:

```Groovy
implementation 'com.google.cloud:google-cloud-spanner:6.66.0'
implementation 'com.google.cloud:google-cloud-spanner:6.67.0'
```

If you are using SBT, add this to your dependencies:

```Scala
libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.66.0"
libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.67.0"
```
<!-- {x-version-update-end} -->

Expand Down Expand Up @@ -671,7 +671,7 @@ Java is a registered trademark of Oracle and/or its affiliates.
[kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java11.html
[stability-image]: https://img.shields.io/badge/stability-stable-green
[maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-spanner.svg
[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.66.0
[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.67.0
[authentication]: https://github.com/googleapis/google-cloud-java#authentication
[auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes
[predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles
Expand Down
28 changes: 26 additions & 2 deletions google-cloud-spanner/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,12 @@
<className>com/google/cloud/spanner/spi/v1/GapicSpannerRpc</className>
<method>com.google.cloud.spanner.spi.v1.SpannerRpc$StreamingCall read(com.google.spanner.v1.ReadRequest, com.google.cloud.spanner.spi.v1.SpannerRpc$ResultStreamConsumer, java.util.Map)</method>
</difference>
<difference>
<differenceType>7005</differenceType>
<className>com/google/cloud/spanner/spi/v1/GapicSpannerRpc</className>
<method>com.google.api.gax.longrunning.OperationFuture updateDatabaseDdl(java.lang.String, java.lang.Iterable, java.lang.String)</method>
<to>com.google.api.gax.longrunning.OperationFuture updateDatabaseDdl(com.google.cloud.spanner.Database, java.lang.Iterable, java.lang.String)</to>
</difference>
<difference>
<differenceType>7005</differenceType>
<className>com/google/cloud/spanner/spi/v1/GapicSpannerRpc</className>
Expand Down Expand Up @@ -387,6 +393,12 @@
<className>com/google/cloud/spanner/spi/v1/SpannerRpc</className>
<method>com.google.cloud.spanner.spi.v1.SpannerRpc$StreamingCall read(com.google.spanner.v1.ReadRequest, com.google.cloud.spanner.spi.v1.SpannerRpc$ResultStreamConsumer, java.util.Map)</method>
</difference>
<difference>
<differenceType>7005</differenceType>
<className>com/google/cloud/spanner/spi/v1/SpannerRpc</className>
<method>com.google.api.gax.longrunning.OperationFuture updateDatabaseDdl(java.lang.String, java.lang.Iterable, java.lang.String)</method>
<to>com.google.api.gax.longrunning.OperationFuture updateDatabaseDdl(com.google.cloud.spanner.Database, java.lang.Iterable, java.lang.String)</to>
</difference>
<difference>
<differenceType>7005</differenceType>
<className>com/google/cloud/spanner/spi/v1/SpannerRpc</className>
Expand Down Expand Up @@ -656,7 +668,7 @@
<className>com/google/cloud/spanner/connection/Connection</className>
<method>com.google.cloud.spanner.Spanner getSpanner()</method>
</difference>

<!-- Add DdlInTransactionMode -->
<difference>
<differenceType>7012</differenceType>
Expand All @@ -668,7 +680,7 @@
<className>com/google/cloud/spanner/connection/Connection</className>
<method>com.google.cloud.spanner.connection.DdlInTransactionMode getDdlInTransactionMode()</method>
</difference>

<!-- Added extended tracing option -->
<difference>
<differenceType>7012</differenceType>
Expand All @@ -688,4 +700,16 @@
<method>void setExcludeTxnFromChangeStreams(boolean)</method>
</difference>

<!-- Added Proto descriptors for proto columns -->
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/connection/Connection</className>
<method>byte[] getProtoDescriptors()</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/connection/Connection</className>
<method>void setProtoDescriptors(byte[])</method>
</difference>

</differences>
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public static Type pgOid() {
/**
* To get the descriptor for the {@code PROTO} type.
*
* @param protoTypeFqn Proto fully qualified name (ex: "spanner.examples.music.SingerInfo").
* @param protoTypeFqn Proto fully qualified name (ex: "examples.spanner.music.SingerInfo").
*/
public static Type proto(String protoTypeFqn) {
return new Type(Code.PROTO, protoTypeFqn);
Expand All @@ -156,7 +156,7 @@ public static Type proto(String protoTypeFqn) {
/**
* To get the descriptor for the {@code ENUM} type.
*
* @param protoTypeFqn Proto ENUM fully qualified name (ex: "spanner.examples.music.Genre")
* @param protoTypeFqn Proto ENUM fully qualified name (ex: "examples.spanner.music.Genre")
*/
public static Type protoEnum(String protoTypeFqn) {
return new Type(Code.ENUM, protoTypeFqn);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@
import com.google.cloud.spanner.connection.PgTransactionMode.IsolationLevel;
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.protobuf.Duration;
import com.google.protobuf.util.Durations;
import com.google.spanner.v1.DirectedReadOptions;
import com.google.spanner.v1.RequestOptions.Priority;
import java.util.Base64;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Locale;
Expand Down Expand Up @@ -563,4 +565,46 @@ public String convert(String value) {
return value.substring(7).trim();
}
}

/** Converter for converting Base64 encoded string to byte[] */
static class ProtoDescriptorsConverter implements ClientSideStatementValueConverter<byte[]> {

public ProtoDescriptorsConverter(String allowedValues) {}

@Override
public Class<byte[]> getParameterClass() {
return byte[].class;
}

@Override
public byte[] convert(String value) {
if (value == null || value.length() == 0 || value.equalsIgnoreCase("null")) {
return null;
}
try {
return Base64.getDecoder().decode(value);
} catch (IllegalArgumentException e) {
return null;
}
}
}

/** Converter for converting String that take in file path as input to String */
static class ProtoDescriptorsFileConverter implements ClientSideStatementValueConverter<String> {

public ProtoDescriptorsFileConverter(String allowedValues) {}

@Override
public Class<String> getParameterClass() {
return String.class;
}

@Override
public String convert(String filePath) {
if (Strings.isNullOrEmpty(filePath)) {
return null;
}
return filePath;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nonnull;

/**
* Internal connection API for Google Cloud Spanner. This interface may introduce breaking changes
Expand Down Expand Up @@ -403,6 +404,25 @@ default boolean isExcludeTxnFromChangeStreams() {
throw new UnsupportedOperationException();
}

/**
* Sets the proto descriptors to use for the next DDL statement (single or batch) that will be
* executed. The proto descriptor is automatically cleared after the statement is executed.
*
* @param protoDescriptors The proto descriptors to use with the next DDL statement (single or
* batch) that will be executed on this connection.
*/
default void setProtoDescriptors(@Nonnull byte[] protoDescriptors) {
throw new UnsupportedOperationException();
}

/**
* @return The proto descriptor that will be used with the next DDL statement (single or batch)
* that is executed on this connection.
*/
default byte[] getProtoDescriptors() {
throw new UnsupportedOperationException();
}

/**
* @return <code>true</code> if this connection will automatically retry read/write transactions
* that abort. This method may only be called when the connection is in read/write
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutures;
import com.google.api.gax.core.GaxProperties;
import com.google.cloud.ByteArray;
import com.google.cloud.Timestamp;
import com.google.cloud.spanner.AsyncResultSet;
import com.google.cloud.spanner.BatchClient;
Expand Down Expand Up @@ -65,6 +66,9 @@
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.Tracer;
import java.io.File;
import java.io.FileInputStream;
import java.io.InputStream;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -81,6 +85,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Instant;

Expand Down Expand Up @@ -113,7 +118,7 @@ private LeakedConnectionException() {
}
}

private volatile LeakedConnectionException leakedException;;
private volatile LeakedConnectionException leakedException;
private final SpannerPool spannerPool;
private AbstractStatementParser statementParser;
/**
Expand Down Expand Up @@ -268,10 +273,11 @@ static UnitOfWorkType of(TransactionMode transactionMode) {

private String transactionTag;
private String statementTag;

private boolean excludeTxnFromChangeStreams;

private Duration maxCommitDelay;
private byte[] protoDescriptors;
private String protoDescriptorsFilePath;

/** Create a connection and register it in the SpannerPool. */
ConnectionImpl(ConnectionOptions options) {
Expand Down Expand Up @@ -353,6 +359,7 @@ public Spanner getSpanner() {
private DdlClient createDdlClient() {
return DdlClient.newBuilder()
.setDatabaseAdminClient(spanner.getDatabaseAdminClient())
.setProjectId(options.getProjectId())
.setInstanceId(options.getInstanceId())
.setDatabaseName(options.getDatabaseName())
.build();
Expand Down Expand Up @@ -763,6 +770,52 @@ public void setExcludeTxnFromChangeStreams(boolean excludeTxnFromChangeStreams)
this.excludeTxnFromChangeStreams = excludeTxnFromChangeStreams;
}

@Override
public byte[] getProtoDescriptors() {
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
if (this.protoDescriptors == null && this.protoDescriptorsFilePath != null) {
// Read from file if filepath is valid
try {
File protoDescriptorsFile = new File(this.protoDescriptorsFilePath);
if (!protoDescriptorsFile.isFile()) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
String.format(
"File %s is not a valid proto descriptors file", this.protoDescriptorsFilePath));
}
InputStream pdStream = new FileInputStream(protoDescriptorsFile);
this.protoDescriptors = ByteArray.copyFrom(pdStream).toByteArray();
} catch (Exception exception) {
throw SpannerExceptionFactory.newSpannerException(exception);
}
}
return this.protoDescriptors;
}

@Override
public void setProtoDescriptors(@Nonnull byte[] protoDescriptors) {
Preconditions.checkNotNull(protoDescriptors);
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
ConnectionPreconditions.checkState(
!isBatchActive(), "Proto descriptors cannot be set when a batch is active");
this.protoDescriptors = protoDescriptors;
this.protoDescriptorsFilePath = null;
}

void setProtoDescriptorsFilePath(@Nonnull String protoDescriptorsFilePath) {
Preconditions.checkNotNull(protoDescriptorsFilePath);
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
ConnectionPreconditions.checkState(
!isBatchActive(), "Proto descriptors file path cannot be set when a batch is active");
this.protoDescriptorsFilePath = protoDescriptorsFilePath;
this.protoDescriptors = null;
}

String getProtoDescriptorsFilePath() {
ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
return this.protoDescriptorsFilePath;
}

/**
* Throws an {@link SpannerException} with code {@link ErrorCode#FAILED_PRECONDITION} if the
* current state of this connection does not allow changing the setting for retryAbortsInternally.
Expand Down Expand Up @@ -1806,6 +1859,7 @@ UnitOfWork createNewUnitOfWork(
.setSpan(
createSpanForUnitOfWork(
statementType == StatementType.DDL ? DDL_STATEMENT : SINGLE_USE_TRANSACTION))
.setProtoDescriptors(getProtoDescriptors())
.build();
if (!isInternalMetadataQuery && !forceSingleUse) {
// Reset the transaction options after starting a single-use transaction.
Expand Down Expand Up @@ -1862,6 +1916,7 @@ UnitOfWork createNewUnitOfWork(
.setStatementTimeout(statementTimeout)
.withStatementExecutor(statementExecutor)
.setSpan(createSpanForUnitOfWork(DDL_BATCH))
.setProtoDescriptors(getProtoDescriptors())
.build();
default:
}
Expand All @@ -1885,7 +1940,11 @@ private void popUnitOfWorkFromTransactionStack() {
}

private ApiFuture<Void> executeDdlAsync(CallType callType, ParsedStatement ddl) {
return getOrStartDdlUnitOfWork().executeDdlAsync(callType, ddl);
ApiFuture<Void> result = getOrStartDdlUnitOfWork().executeDdlAsync(callType, ddl);
// reset proto descriptors after executing a DDL statement
this.protoDescriptors = null;
this.protoDescriptorsFilePath = null;
Comment on lines +1944 to +1946
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to move this to setDefaultTransactionOptions()? It keeps the code cleaner, as we have all the reset code in one method, but it does mean that executing for example a query after setting the proto descriptors also clears the fields. One option to prevent the latter could be to add an input argument to setDefaultTransactionOptions() that indicates the type of statement that was executed, and only reset these fields if the last statement was a DDL statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will check if it makes sense to move that logic to setDefaultTransactionOptions() as a seperate PR. This PR is very big that the code review will be difficult. Will work on this immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment will be addressed in PR #3126

return result;
}

@Override
Expand Down Expand Up @@ -1985,6 +2044,11 @@ public ApiFuture<long[]> runBatchAsync() {
}
return ApiFutures.immediateFuture(new long[0]);
} finally {
if (isDdlBatchActive()) {
// reset proto descriptors after executing a DDL batch
this.protoDescriptors = null;
this.protoDescriptorsFilePath = null;
}
this.batchMode = BatchMode.NONE;
setDefaultTransactionOptions();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,14 @@ StatementResult statementSetPgSessionCharacteristicsTransactionMode(

StatementResult statementShowTransactionIsolationLevel();

StatementResult statementSetProtoDescriptors(byte[] protoDescriptors);

StatementResult statementSetProtoDescriptorsFilePath(String filePath);

StatementResult statementShowProtoDescriptors();

StatementResult statementShowProtoDescriptorsFilePath();

StatementResult statementExplain(String sql);

StatementResult statementShowDataBoostEnabled();
Expand Down
Loading
Loading