-
Notifications
You must be signed in to change notification settings - Fork 26
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
fix: add support for arrays in ResultSets #36
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
592f779
fix: add support for arrays in ResultSets
olavloite 71eaeb2
Merge branch 'postgresql-dialect' into arrays-resultset
olavloite d29d931
Merge branch 'postgresql-dialect' into arrays-resultset
Vizerai 4190f12
Merge branch 'postgresql-dialect' into arrays-resultset
olavloite c1c7174
Merge branch 'arrays-resultset' of github.com:GoogleCloudPlatform/pga…
olavloite 30443ae
Merge branch 'postgresql-dialect' into arrays-resultset
olavloite 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
There are no files selected for viewing
This file contains 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 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 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 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
245 changes: 245 additions & 0 deletions
245
src/test/java/com/google/cloud/spanner/pgadapter/AbstractMockServerTest.java
This file contains 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 |
|---|---|---|
| @@ -0,0 +1,245 @@ | ||
| // Copyright 2022 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.google.cloud.spanner.pgadapter; | ||
|
|
||
| import com.google.api.gax.core.NoCredentialsProvider; | ||
| import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; | ||
| import com.google.api.gax.longrunning.OperationSnapshot; | ||
| import com.google.api.gax.longrunning.OperationTimedPollAlgorithm; | ||
| import com.google.api.gax.retrying.RetrySettings; | ||
| import com.google.api.gax.rpc.UnaryCallSettings; | ||
| import com.google.cloud.NoCredentials; | ||
| import com.google.cloud.spanner.ErrorCode; | ||
| import com.google.cloud.spanner.MockDatabaseAdminServiceImpl; | ||
| import com.google.cloud.spanner.MockOperationsServiceImpl; | ||
| import com.google.cloud.spanner.MockSpannerServiceImpl; | ||
| import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult; | ||
| import com.google.cloud.spanner.SpannerException; | ||
| import com.google.cloud.spanner.Statement; | ||
| import com.google.cloud.spanner.admin.database.v1.DatabaseAdminClient; | ||
| import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; | ||
| import com.google.cloud.spanner.connection.SpannerPool; | ||
| import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata; | ||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.protobuf.ListValue; | ||
| import com.google.protobuf.Value; | ||
| import com.google.spanner.admin.database.v1.CreateDatabaseRequest; | ||
| import com.google.spanner.admin.database.v1.InstanceName; | ||
| import com.google.spanner.v1.ResultSetMetadata; | ||
| import com.google.spanner.v1.StructType; | ||
| import com.google.spanner.v1.StructType.Field; | ||
| import com.google.spanner.v1.Type; | ||
| import com.google.spanner.v1.TypeCode; | ||
| import io.grpc.ManagedChannelBuilder; | ||
| import io.grpc.Server; | ||
| import io.grpc.netty.shaded.io.grpc.netty.NettyServerBuilder; | ||
| import java.net.InetSocketAddress; | ||
| import java.util.logging.Logger; | ||
| import org.junit.AfterClass; | ||
| import org.junit.Before; | ||
| import org.junit.BeforeClass; | ||
| import org.threeten.bp.Duration; | ||
|
|
||
| /** | ||
| * Abstract base class for tests that verify that PgAdapter is receiving wire protocol requests | ||
| * correctly, translates these correctly to Spanner RPC invocations, and correctly translates the | ||
| * RPC invocations back to wire protocol responses. The test starts two in-process servers for this | ||
| * purpose: 1. An in-process {@link MockSpannerServiceImpl}. The mock server implements the entire | ||
| * gRPC API of Cloud Spanner, but does not contain an actual query engine or any other | ||
| * implementation. Instead, all query and DML statements that the client will be executing must | ||
| * first be registered as mock results on the server. This makes the mock server dialect agnostic | ||
| * and usable for both normal Spanner requests and Spangres requests. Note that this also means that | ||
| * the server does NOT verify that the SQL statement is correct and valid for the specific dialect. | ||
| * It only verifies that the statement corresponds with one of the previously registered statements | ||
| * on the server. 2. An in-process PgAdapter {@link ProxyServer} that connects to the | ||
| * above-mentioned mock Spanner server. The in-process PgAdapter server listens on a random local | ||
| * port, and tests can use the client of their choosing to connect to the {@link ProxyServer}. The | ||
| * requests are translated by the proxy into RPC invocations on the mock Spanner server, and the | ||
| * responses from the mock Spanner server are translated into wire protocol responses to the client. | ||
| * The tests can then inspect the requests that the mock Spanner server received to verify that the | ||
| * server received the requests that the test expected. | ||
| */ | ||
| abstract class AbstractMockServerTest { | ||
| private static final Logger logger = Logger.getLogger(AbstractMockServerTest.class.getName()); | ||
|
|
||
| protected static final Statement SELECT1 = Statement.of("SELECT 1"); | ||
| protected static final Statement SELECT2 = Statement.of("SELECT 2"); | ||
| private static final ResultSetMetadata SELECT1_METADATA = | ||
| ResultSetMetadata.newBuilder() | ||
| .setRowType( | ||
| StructType.newBuilder() | ||
| .addFields( | ||
| Field.newBuilder() | ||
| .setName("C") | ||
| .setType(Type.newBuilder().setCode(TypeCode.INT64).build()) | ||
| .build()) | ||
| .build()) | ||
| .build(); | ||
| private static final com.google.spanner.v1.ResultSet SELECT1_RESULTSET = | ||
| com.google.spanner.v1.ResultSet.newBuilder() | ||
| .addRows( | ||
| ListValue.newBuilder() | ||
| .addValues(Value.newBuilder().setStringValue("1").build()) | ||
| .build()) | ||
| .setMetadata(SELECT1_METADATA) | ||
| .build(); | ||
| private static final com.google.spanner.v1.ResultSet SELECT2_RESULTSET = | ||
| com.google.spanner.v1.ResultSet.newBuilder() | ||
| .addRows( | ||
| ListValue.newBuilder() | ||
| .addValues(Value.newBuilder().setStringValue("2").build()) | ||
| .build()) | ||
| .setMetadata(SELECT1_METADATA) | ||
| .build(); | ||
| protected static final Statement UPDATE_STATEMENT = | ||
| Statement.of("UPDATE FOO SET BAR=1 WHERE BAZ=2"); | ||
| protected static final int UPDATE_COUNT = 2; | ||
| protected static final Statement INSERT_STATEMENT = Statement.of("INSERT INTO FOO VALUES (1)"); | ||
| protected static final int INSERT_COUNT = 1; | ||
|
|
||
| protected static MockSpannerServiceImpl mockSpanner; | ||
| protected static MockOperationsServiceImpl mockOperationsService; | ||
| protected static MockDatabaseAdminServiceImpl mockDatabaseAdmin; | ||
| private static Server spannerServer; | ||
| protected static ProxyServer pgServer; | ||
|
|
||
| @BeforeClass | ||
| public static void startMockSpannerAndPgAdapterServers() throws Exception { | ||
| mockSpanner = new MockSpannerServiceImpl(); | ||
| mockSpanner.setAbortProbability(0.0D); // We don't want any unpredictable aborted transactions. | ||
| mockSpanner.putStatementResult(StatementResult.query(SELECT1, SELECT1_RESULTSET)); | ||
| mockSpanner.putStatementResult(StatementResult.query(SELECT2, SELECT2_RESULTSET)); | ||
| mockSpanner.putStatementResult(StatementResult.update(UPDATE_STATEMENT, UPDATE_COUNT)); | ||
| mockSpanner.putStatementResult(StatementResult.update(INSERT_STATEMENT, INSERT_COUNT)); | ||
|
|
||
| mockOperationsService = new MockOperationsServiceImpl(); | ||
| mockDatabaseAdmin = new MockDatabaseAdminServiceImpl(mockOperationsService); | ||
|
|
||
| InetSocketAddress address = new InetSocketAddress("localhost", 0); | ||
| spannerServer = | ||
| NettyServerBuilder.forAddress(address) | ||
| .addService(mockSpanner) | ||
| .addService(mockDatabaseAdmin) | ||
| .addService(mockOperationsService) | ||
| .build() | ||
| .start(); | ||
|
|
||
| // Create the test database on the mock server. This should be replaced by a simple feature in | ||
| // the mock server to just add a database instead of having to simulate the creation of it. | ||
| createDatabase(); | ||
|
|
||
| ImmutableList.Builder<String> argsListBuilder = | ||
| ImmutableList.<String>builder() | ||
| .add( | ||
| "-p", | ||
| "p", | ||
| "-i", | ||
| "i", | ||
| "-d", | ||
| "d", | ||
| "-c", | ||
| "", // empty credentials file, as we are using a plain text connection. | ||
| "-s", | ||
| "0", // port 0 to let the OS pick an available port | ||
| "-e", | ||
| String.format("localhost:%d", spannerServer.getPort()), | ||
| "-r", | ||
| "usePlainText=true;"); | ||
| String[] args = argsListBuilder.build().toArray(new String[0]); | ||
| pgServer = new ProxyServer(new OptionsMetadata(args)); | ||
| pgServer.startServer(); | ||
| } | ||
|
|
||
| private static void createDatabase() throws Exception { | ||
| // TODO: Replace this entire method with a feature in the test framework to just manually add a | ||
| // database instead of having to create it. | ||
| DatabaseAdminSettings.Builder builder = | ||
| DatabaseAdminSettings.newBuilder() | ||
| .setCredentialsProvider(NoCredentialsProvider.create()) | ||
| .setTransportChannelProvider( | ||
| InstantiatingGrpcChannelProvider.newBuilder() | ||
| .setEndpoint(String.format("localhost:%d", spannerServer.getPort())) | ||
| .setCredentials(NoCredentials.getInstance()) | ||
| .setChannelConfigurator(ManagedChannelBuilder::usePlaintext) | ||
| .build()); | ||
|
|
||
| RetrySettings longRunningInitialRetrySettings = | ||
| RetrySettings.newBuilder() | ||
| .setInitialRpcTimeout(Duration.ofMillis(600L)) | ||
| .setMaxRpcTimeout(Duration.ofMillis(6000L)) | ||
| .setInitialRetryDelay(Duration.ofMillis(20L)) | ||
| .setMaxRetryDelay(Duration.ofMillis(45L)) | ||
| .setRetryDelayMultiplier(1.5) | ||
| .setRpcTimeoutMultiplier(1.5) | ||
| .setTotalTimeout(Duration.ofMinutes(48L)) | ||
| .build(); | ||
| builder | ||
| .createDatabaseOperationSettings() | ||
| .setInitialCallSettings( | ||
| UnaryCallSettings | ||
| .<CreateDatabaseRequest, OperationSnapshot>newUnaryCallSettingsBuilder() | ||
| .setRetrySettings(longRunningInitialRetrySettings) | ||
| .build()); | ||
| builder | ||
| .createDatabaseOperationSettings() | ||
| .setPollingAlgorithm( | ||
| OperationTimedPollAlgorithm.create( | ||
| RetrySettings.newBuilder() | ||
| .setInitialRpcTimeout(Duration.ofMillis(20L)) | ||
| .setInitialRetryDelay(Duration.ofMillis(10L)) | ||
| .setMaxRetryDelay(Duration.ofMillis(150L)) | ||
| .setMaxRpcTimeout(Duration.ofMillis(150L)) | ||
| .setMaxAttempts(10) | ||
| .setTotalTimeout(Duration.ofMillis(5000L)) | ||
| .setRetryDelayMultiplier(1.3) | ||
| .setRpcTimeoutMultiplier(1.3) | ||
| .build())); | ||
|
|
||
| DatabaseAdminClient client = DatabaseAdminClient.create(builder.build()); | ||
| client | ||
| .createDatabaseAsync( | ||
| InstanceName.newBuilder().setProject("p").setInstance("i").build(), "CREATE DATABASE d") | ||
| .get(); | ||
| client.close(); | ||
| } | ||
|
|
||
| @AfterClass | ||
| public static void stopMockSpannerAndPgAdapterServers() throws Exception { | ||
| pgServer.stopServer(); | ||
| try { | ||
| SpannerPool.closeSpannerPool(); | ||
| } catch (SpannerException e) { | ||
| if (e.getErrorCode() == ErrorCode.FAILED_PRECONDITION | ||
| && e.getMessage() | ||
| .contains( | ||
| "There is/are 1 connection(s) still open. Close all connections before calling closeSpanner()")) { | ||
| // Ignore this exception for now. It is caused by the fact that the PgAdapter proxy server | ||
| // is not gracefully shutting down all connections when the proxy is stopped, and it also | ||
| // does not wait until any connections that have been requested to close, actually have | ||
| // closed. | ||
| logger.warning(String.format("Ignoring %s as this is expected", e.getMessage())); | ||
| } else { | ||
| throw e; | ||
| } | ||
| } | ||
| spannerServer.shutdown(); | ||
| spannerServer.awaitTermination(); | ||
| } | ||
|
|
||
| @Before | ||
| public void clearRequests() { | ||
| mockSpanner.clearRequests(); | ||
| } | ||
| } |
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.
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.
Don't we want the PG version of the library?
6.17.4-pg-SNAPSHOT
I don't believe the PG changes have been rolled into mainline yet.
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.
This is the test-lib that contains the mock Spanner server. That is (AFAIK) not included in the PG snapshot release. So this will only be used for test and starting the mock Spanner server.
That being said, the PG feature has now been merged into the Java client library and released, so I'll update this PR to use that version instead.
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.
Hmm... I tried using the newest version of the Spanner testlib locally, but that does not work because of the API changes between 6.17.4 and 6.19.0. So we are better off using the 6.17.4 testlib here, and then update everything once the JDBC driver also has been updated to use 6.19.0 (or we have gotten rid of the JDBC driver altogether).
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.
We pull in the jdbc library 2.5.7-pg-SNAPSHOT which then pulls in the pg version of the client library 6.17.4-pg-SNAPSHOT.
Won't this lead to some problems by importing both versions of the library?
6.17.4-pg-SNAPSHOT
6.17.4
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.
That's a good point. They are actually importing two different things:
spanner-6.17.4-pg-SNAPSHOT. This is the main library (basically everything insrc/main/javaof the library).<type>testlib</type>which means that it only imports the test code of the library (basically everything insrc/test/javaof the library)The above allows us to reuse test infrastructure from the client library. As it is imported with
<scope>test</scope>it will also not be included in the build of PgAdapter, so it is not shipped to customers.We actually do the same in the JDBC driver itself: