This repository was archived by the owner on Apr 7, 2026. It is now read-only.
feat: add support for Proto Columns to Connection API #2495
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,9 +25,11 @@ | |
| 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.RequestOptions.Priority; | ||
| import java.util.Base64; | ||
| import java.util.EnumSet; | ||
| import java.util.HashMap; | ||
| import java.util.Locale; | ||
|
|
@@ -494,4 +496,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; | ||
|
harshachinta marked this conversation as resolved.
|
||
| } | ||
| } | ||
| } | ||
|
|
||
| /** Converter for converting String that take in file path as input to String */ | ||
| static class ProtoDescriptorsFileConverter implements ClientSideStatementValueConverter<String> { | ||
|
Contributor
There was a problem hiding this comment. We supporting reading from the file here? If Yes where should file be located just on resources or local path ?
Contributor
Author
There was a problem hiding this comment. Yes we are supporting reading from a file path given through a client side statement. This is needed to support ORM's and other frameworks. This discussion is available in the design doc. |
||
|
|
||
| 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; | ||
|
harshachinta marked this conversation as resolved.
|
||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
|
|
||
| import com.google.api.core.ApiFuture; | ||
| import com.google.api.core.ApiFutures; | ||
| import com.google.cloud.ByteArray; | ||
| import com.google.cloud.Timestamp; | ||
| import com.google.cloud.spanner.AsyncResultSet; | ||
| import com.google.cloud.spanner.CommitResponse; | ||
|
|
@@ -51,6 +52,9 @@ | |
| import com.google.common.util.concurrent.MoreExecutors; | ||
| import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; | ||
| import com.google.spanner.v1.ResultSetStats; | ||
| import java.io.File; | ||
| import java.io.FileInputStream; | ||
| import java.io.InputStream; | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
|
|
@@ -63,6 +67,7 @@ | |
| import java.util.concurrent.ThreadFactory; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.TimeoutException; | ||
| import javax.annotation.Nonnull; | ||
| import org.threeten.bp.Instant; | ||
|
|
||
| /** Implementation for {@link Connection}, the generic Spanner connection API (not JDBC). */ | ||
|
|
@@ -85,7 +90,7 @@ private LeakedConnectionException() { | |
| } | ||
| } | ||
|
|
||
| private volatile LeakedConnectionException leakedException; | ||
| private final SpannerPool spannerPool; | ||
| private AbstractStatementParser statementParser; | ||
| /** | ||
|
|
@@ -221,6 +226,9 @@ static UnitOfWorkType of(TransactionMode transactionMode) { | |
| private String transactionTag; | ||
| private String statementTag; | ||
|
|
||
| private byte[] protoDescriptors; | ||
| private String protoDescriptorsFilePath; | ||
|
|
||
| /** Create a connection and register it in the SpannerPool. */ | ||
| ConnectionImpl(ConnectionOptions options) { | ||
| Preconditions.checkNotNull(options); | ||
|
|
@@ -278,6 +286,7 @@ Spanner getSpanner() { | |
| private DdlClient createDdlClient() { | ||
| return DdlClient.newBuilder() | ||
| .setDatabaseAdminClient(spanner.getDatabaseAdminClient()) | ||
| .setProjectId(options.getProjectId()) | ||
| .setInstanceId(options.getInstanceId()) | ||
| .setDatabaseName(options.getDatabaseName()) | ||
| .build(); | ||
|
|
@@ -623,6 +632,52 @@ public void setStatementTag(String tag) { | |
| this.statementTag = tag; | ||
| } | ||
|
|
||
| @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()) { | ||
|
harshachinta marked this conversation as resolved.
|
||
| 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; | ||
|
harshachinta marked this conversation as resolved.
|
||
| 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. | ||
|
|
@@ -1379,6 +1434,7 @@ UnitOfWork createNewUnitOfWork() { | |
| .setReturnCommitStats(returnCommitStats) | ||
| .setStatementTimeout(statementTimeout) | ||
| .withStatementExecutor(statementExecutor) | ||
| .setProtoDescriptors(getProtoDescriptors()) | ||
| .build(); | ||
| } else { | ||
| switch (getUnitOfWorkType()) { | ||
|
|
@@ -1421,6 +1477,7 @@ UnitOfWork createNewUnitOfWork() { | |
| .setDatabaseClient(dbClient) | ||
| .setStatementTimeout(statementTimeout) | ||
| .withStatementExecutor(statementExecutor) | ||
| .setProtoDescriptors(getProtoDescriptors()) | ||
| .build(); | ||
| default: | ||
| } | ||
|
|
@@ -1444,7 +1501,12 @@ private void popUnitOfWorkFromTransactionStack() { | |
| } | ||
|
|
||
| private ApiFuture<Void> executeDdlAsync(CallType callType, ParsedStatement ddl) { | ||
| ApiFuture<Void> result = | ||
| getCurrentUnitOfWorkOrStartNewUnitOfWork().executeDdlAsync(callType, ddl); | ||
| // reset proto descriptors after executing a DDL statement | ||
| this.protoDescriptors = null; | ||
|
harshachinta marked this conversation as resolved.
|
||
| this.protoDescriptorsFilePath = null; | ||
| return result; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -1535,6 +1597,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; | ||
|
harshachinta marked this conversation as resolved.
|
||
| this.protoDescriptorsFilePath = null; | ||
| } | ||
| this.batchMode = BatchMode.NONE; | ||
| setDefaultTransactionOptions(); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,5 +124,13 @@ StatementResult statementSetPgSessionCharacteristicsTransactionMode( | |
|
|
||
| StatementResult statementShowTransactionIsolationLevel(); | ||
|
|
||
| StatementResult statementSetProtoDescriptors(byte[] protoDescriptors); | ||
|
|
||
| StatementResult statementSetProtoDescriptorsFilePath(String filePath); | ||
|
|
||
| StatementResult statementShowProtoDescriptors(); | ||
|
|
||
| StatementResult statementShowProtoDescriptorsFilePath(); | ||
|
|
||
| StatementResult statementExplain(String sql); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,8 @@ | |
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_DELAY_TRANSACTION_START_UNTIL_FIRST_WRITE; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_OPTIMIZER_STATISTICS_PACKAGE; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_OPTIMIZER_VERSION; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_PROTO_DESCRIPTORS; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_PROTO_DESCRIPTORS_FILE_PATH; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_READONLY; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_READ_ONLY_STALENESS; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SET_RETRY_ABORTS_INTERNALLY; | ||
|
|
@@ -45,6 +47,8 @@ | |
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_DELAY_TRANSACTION_START_UNTIL_FIRST_WRITE; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_OPTIMIZER_STATISTICS_PACKAGE; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_OPTIMIZER_VERSION; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_PROTO_DESCRIPTORS; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_PROTO_DESCRIPTORS_FILE_PATH; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_READONLY; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_READ_ONLY_STALENESS; | ||
| import static com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType.SHOW_READ_TIMESTAMP; | ||
|
|
@@ -493,6 +497,36 @@ public StatementResult statementShowTransactionIsolationLevel() { | |
| return resultSet("transaction_isolation", "serializable", SHOW_TRANSACTION_ISOLATION_LEVEL); | ||
| } | ||
|
|
||
| @Override | ||
| public StatementResult statementSetProtoDescriptors(byte[] protoDescriptors) { | ||
| Preconditions.checkNotNull(protoDescriptors); | ||
| getConnection().setProtoDescriptors(protoDescriptors); | ||
| return noResult(SET_PROTO_DESCRIPTORS); | ||
| } | ||
|
|
||
| @Override | ||
| public StatementResult statementSetProtoDescriptorsFilePath(String filePath) { | ||
| Preconditions.checkNotNull(filePath); | ||
| getConnection().setProtoDescriptorsFilePath(filePath); | ||
| return noResult(SET_PROTO_DESCRIPTORS_FILE_PATH); | ||
| } | ||
|
|
||
| @Override | ||
| public StatementResult statementShowProtoDescriptors() { | ||
|
harshachinta marked this conversation as resolved.
|
||
| return resultSet( | ||
| String.format("%sPROTO_DESCRIPTORS", getNamespace(connection.getDialect())), | ||
| getConnection().getProtoDescriptors(), | ||
| SHOW_PROTO_DESCRIPTORS); | ||
| } | ||
|
|
||
| @Override | ||
| public StatementResult statementShowProtoDescriptorsFilePath() { | ||
| return resultSet( | ||
| String.format("%sPROTO_DESCRIPTORS_FILE_PATH", getNamespace(connection.getDialect())), | ||
| getConnection().getProtoDescriptorsFilePath(), | ||
| SHOW_PROTO_DESCRIPTORS_FILE_PATH); | ||
| } | ||
|
|
||
| private String processQueryPlan(PlanNode planNode) { | ||
| StringBuilder planNodeDescription = new StringBuilder(" : { "); | ||
| com.google.protobuf.Struct metadata = planNode.getMetadata(); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,10 +58,12 @@ class DdlBatch extends AbstractBaseUnitOfWork { | |
| private final DatabaseClient dbClient; | ||
| private final List<String> statements = new ArrayList<>(); | ||
| private UnitOfWorkState state = UnitOfWorkState.STARTED; | ||
| private final byte[] protoDescriptors; | ||
|
|
||
| static class Builder extends AbstractBaseUnitOfWork.Builder<Builder, DdlBatch> { | ||
| private DdlClient ddlClient; | ||
| private DatabaseClient dbClient; | ||
| private byte[] protoDescriptors; | ||
|
|
||
| private Builder() {} | ||
|
|
||
|
|
@@ -77,6 +79,11 @@ Builder setDatabaseClient(DatabaseClient client) { | |
| return this; | ||
| } | ||
|
|
||
| Builder setProtoDescriptors(byte[] protoDescriptors) { | ||
| this.protoDescriptors = protoDescriptors; | ||
| return this; | ||
| } | ||
|
|
||
| @Override | ||
| DdlBatch build() { | ||
| Preconditions.checkState(ddlClient != null, "No DdlClient specified"); | ||
|
|
@@ -93,6 +100,7 @@ private DdlBatch(Builder builder) { | |
| super(builder); | ||
| this.ddlClient = builder.ddlClient; | ||
| this.dbClient = builder.dbClient; | ||
| this.protoDescriptors = builder.protoDescriptors; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -239,7 +247,7 @@ public ApiFuture<long[]> runBatchAsync(CallType callType) { | |
| () -> { | ||
| try { | ||
| OperationFuture<Void, UpdateDatabaseDdlMetadata> operation = | ||
| ddlClient.executeDdl(statements, protoDescriptors); | ||
| try { | ||
| // Wait until the operation has finished. | ||
| getWithStatementTimeout(operation, RUN_BATCH_STATEMENT); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also add a
NonNullannotation to the argument here. Similarly in other places wherever a null value is not expected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These convert methods are not used by customers and is getting called from link. We generally use this annotation to warn users when they pass
null. But here it is used in code internally and as users don't use this function, I guess it is not needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is being called from the same package and not meant to be public to customers. Then, should we make the methods package-protected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods cannot be package-private, because they implement a method from an interface. The class itself is package-private, which protects the class and all its methods from being used by 'outside' users.