Skip to content

Commit

Permalink
refactor: use analyze to get statement params (#478)
Browse files Browse the repository at this point in the history
* refactor: use analyze to get statement params

* feat: use analyze to describe statements

* fix: get param index from param name instead of index in list

* test: add more tests

* test: add more tests + disable clirr

Disabling clirr as it only adds noise, and somehow refuses to pick up
the latest ignore.

* chore: cleanup

* fix: update test for analyzeUpdate

* fix: disable flaky assertion

* fix: remove accidentally committed folder

* fix: remove more assertions pending node-postgres 8.9

* fix: disable more assertions awaiting 8.9
  • Loading branch information
olavloite authored Nov 25, 2022
1 parent ef49d74 commit 76c14c5
Show file tree
Hide file tree
Showing 40 changed files with 1,831 additions and 1,645 deletions.
10 changes: 0 additions & 10 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ jobs:
distribution: 'zulu'
- run: java -version
- run: .ci/run-with-credentials.sh lint
clirr:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-java@v3
with:
java-version: 8
distribution: 'zulu'
- run: java -version
- run: .ci/run-with-credentials.sh clirr
e2e-psql-v11-v1:
needs: [check-env]
if: needs.check-env.outputs.has-key == 'true'
Expand Down
14 changes: 14 additions & 0 deletions clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,20 @@
<method>void connectToSpanner(java.lang.String, com.google.auth.oauth2.GoogleCredentials)</method>
<to>void connectToSpanner(java.lang.String, com.google.auth.Credentials)</to>
</difference>
<!-- Use backend parsing of statements for describing parameters. -->
<difference>
<differenceType>7005</differenceType>
<className>com/google/cloud/spanner/pgadapter/ConnectionHandler</className>
<method>void registerAutoDescribedStatement(java.lang.String, int[])</method>
<to>void registerAutoDescribedStatement(java.lang.String, java.util.concurrent.Future)</to>
</difference>
<difference>
<differenceType>7006</differenceType>
<className>com/google/cloud/spanner/pgadapter/ConnectionHandler</className>
<method>int[] getAutoDescribedStatement(java.lang.String)</method>
<from>int[]</from>
<to>java.util.concurrent.Future</to>
</difference>

<!-- Add ignores for all sub packages, as these are considered internal. -->
<difference>
Expand Down
7 changes: 6 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
com.google.cloud.spanner.pgadapter.nodejs.NodeJSTest
</excludedTests>

<spanner.version>6.31.2</spanner.version>
<spanner.version>6.33.0</spanner.version>
<junixsocket.version>2.6.1</junixsocket.version>
</properties>

Expand Down Expand Up @@ -72,6 +72,11 @@
</licenses>

<dependencies>
<dependency>
<groupId>com.google.auto.value</groupId>
<artifactId>auto-value-annotations</artifactId>
<version>1.10</version>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-spanner</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.google.cloud.spanner.pgadapter.error.SQLState;
import com.google.cloud.spanner.pgadapter.error.Severity;
import com.google.cloud.spanner.pgadapter.metadata.ConnectionMetadata;
import com.google.cloud.spanner.pgadapter.metadata.DescribeResult;
import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata.SslMode;
import com.google.cloud.spanner.pgadapter.statements.CopyStatement;
Expand Down Expand Up @@ -70,6 +71,7 @@
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.logging.Level;
Expand All @@ -95,7 +97,7 @@ public class ConnectionHandler extends Thread {
private final ProxyServer server;
private Socket socket;
private final Map<String, IntermediatePreparedStatement> statementsMap = new HashMap<>();
private final Cache<String, int[]> autoDescribedStatementsCache =
private final Cache<String, Future<DescribeResult>> autoDescribedStatementsCache =
CacheBuilder.newBuilder()
.expireAfterWrite(Duration.ofMinutes(30L))
.maximumSize(5000L)
Expand Down Expand Up @@ -587,13 +589,13 @@ public void registerStatement(String statementName, IntermediatePreparedStatemen
* Returns the parameter types of a cached auto-described statement, or null if none is available
* in the cache.
*/
public int[] getAutoDescribedStatement(String sql) {
public Future<DescribeResult> getAutoDescribedStatement(String sql) {
return this.autoDescribedStatementsCache.getIfPresent(sql);
}

/** Stores the parameter types of an auto-described statement in the cache. */
public void registerAutoDescribedStatement(String sql, int[] parameterTypes) {
this.autoDescribedStatementsCache.put(sql, parameterTypes);
public void registerAutoDescribedStatement(String sql, Future<DescribeResult> describeResult) {
this.autoDescribedStatementsCache.put(sql, describeResult);
}

public void closeStatement(String statementName) {
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// 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.metadata;

import com.google.api.core.InternalApi;
import com.google.cloud.spanner.ResultSet;
import com.google.cloud.spanner.pgadapter.error.PGExceptionFactory;
import com.google.cloud.spanner.pgadapter.error.SQLState;
import com.google.cloud.spanner.pgadapter.parsers.Parser;
import com.google.spanner.v1.StructType;
import java.util.Arrays;

@InternalApi
public class DescribeResult {
private final ResultSet resultSet;
private final int[] parameters;

public DescribeResult(int[] givenParameterTypes, ResultSet resultMetadata) {
this.resultSet = resultMetadata;
this.parameters = extractParameters(givenParameterTypes, resultMetadata);
}

static int[] extractParameters(int[] givenParameterTypes, ResultSet resultSet) {
if (!resultSet.getMetadata().hasUndeclaredParameters()) {
return givenParameterTypes;
}
return extractParameterTypes(
givenParameterTypes, resultSet.getMetadata().getUndeclaredParameters());
}

static int[] extractParameterTypes(int[] givenParameterTypes, StructType parameters) {
int[] result;
int maxParamIndex = maxParamNumber(parameters);
if (maxParamIndex == givenParameterTypes.length) {
result = givenParameterTypes;
} else {
result =
Arrays.copyOf(givenParameterTypes, Math.max(givenParameterTypes.length, maxParamIndex));
}
for (int i = 0; i < parameters.getFieldsCount(); i++) {
// Only override parameter types that were not specified by the frontend.
int paramIndex = Integer.parseInt(parameters.getFields(i).getName().substring(1)) - 1;
if (paramIndex >= givenParameterTypes.length || givenParameterTypes[paramIndex] == 0) {
result[paramIndex] = Parser.toOid(parameters.getFields(i).getType());
}
}
return result;
}

static int maxParamNumber(StructType parameters) {
int max = 0;
for (int i = 0; i < parameters.getFieldsCount(); i++) {
try {
int paramIndex = Integer.parseInt(parameters.getFields(i).getName().substring(1));
if (paramIndex > max) {
max = paramIndex;
}
} catch (NumberFormatException numberFormatException) {
throw PGExceptionFactory.newPGException(
"Invalid parameter name: " + parameters.getFields(i).getName(), SQLState.InternalError);
}
}
return max;
}

public int[] getParameters() {
return parameters;
}

public ResultSet getResultSet() {
return resultSet;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import com.google.cloud.spanner.Type;
import com.google.cloud.spanner.Type.Code;
import com.google.cloud.spanner.pgadapter.ProxyServer.DataFormat;
import com.google.cloud.spanner.pgadapter.error.PGExceptionFactory;
import com.google.cloud.spanner.pgadapter.error.SQLState;
import com.google.cloud.spanner.pgadapter.session.SessionState;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -263,6 +265,64 @@ public static int toOid(Type type) {
ErrorCode.INVALID_ARGUMENT, "Unsupported or unknown type: " + type);
}
}
/**
* Translates the given Cloud Spanner {@link Type} to a PostgreSQL OID constant.
*
* @param type the type to translate
* @return The OID constant value for the type
*/
public static int toOid(com.google.spanner.v1.Type type) {
switch (type.getCode()) {
case BOOL:
return Oid.BOOL;
case INT64:
return Oid.INT8;
case NUMERIC:
return Oid.NUMERIC;
case FLOAT64:
return Oid.FLOAT8;
case STRING:
return Oid.VARCHAR;
case JSON:
return Oid.JSONB;
case BYTES:
return Oid.BYTEA;
case TIMESTAMP:
return Oid.TIMESTAMPTZ;
case DATE:
return Oid.DATE;
case ARRAY:
switch (type.getArrayElementType().getCode()) {
case BOOL:
return Oid.BOOL_ARRAY;
case INT64:
return Oid.INT8_ARRAY;
case NUMERIC:
return Oid.NUMERIC_ARRAY;
case FLOAT64:
return Oid.FLOAT8_ARRAY;
case STRING:
return Oid.VARCHAR_ARRAY;
case JSON:
return Oid.JSONB_ARRAY;
case BYTES:
return Oid.BYTEA_ARRAY;
case TIMESTAMP:
return Oid.TIMESTAMPTZ_ARRAY;
case DATE:
return Oid.DATE_ARRAY;
case ARRAY:
case STRUCT:
default:
throw PGExceptionFactory.newPGException(
"Unsupported or unknown array type: " + type, SQLState.InternalError);
}
case STRUCT:
default:
throw PGExceptionFactory.newPGException(
"Unsupported or unknown type: " + type, SQLState.InternalError);
}
}

/** Returns the item helder by this parser. */
public T getItem() {
Expand Down
Loading

0 comments on commit 76c14c5

Please sign in to comment.