Skip to content

Commit

Permalink
feat: Support Describe message for DDL statements and other no-result…
Browse files Browse the repository at this point in the history
… statements (#501)

* 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

* cleanup: remove 'guess types' feature

Guessing the type of a parameter is no longer needed with backend
parameter type inferring support.

Fixes #487

* feat: support DML RETURNING clause

Add support for DML statements with a RETURNING clause.

* chore: remove unnecessary change

* feat: support Describe for non-query and DML statements

* test: describe DDL statements
  • Loading branch information
olavloite committed Nov 30, 2022
1 parent 800b9f5 commit cb616d8
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,20 @@
import com.google.cloud.spanner.pgadapter.parsers.Parser;
import com.google.spanner.v1.StructType;
import java.util.Arrays;
import javax.annotation.Nullable;

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

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

static int[] extractParameters(int[] givenParameterTypes, ResultSet resultSet) {
if (!resultSet.getMetadata().hasUndeclaredParameters()) {
static int[] extractParameters(int[] givenParameterTypes, @Nullable ResultSet resultSet) {
if (resultSet == null || !resultSet.getMetadata().hasUndeclaredParameters()) {
return givenParameterTypes;
}
return extractParameterTypes(
Expand Down Expand Up @@ -79,7 +80,7 @@ public int[] getParameters() {
return parameters;
}

public ResultSet getResultSet() {
public @Nullable ResultSet getResultSet() {
return resultSet;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,11 @@ void execute() {
} else if (statement.getSql().isEmpty()) {
result.set(NO_RESULT);
} else if (parsedStatement.isDdl()) {
result.set(ddlExecutor.execute(parsedStatement, statement));
if (analyze) {
result.set(NO_RESULT);
} else {
result.set(ddlExecutor.execute(parsedStatement, statement));
}
} else {
// Potentially replace pg_catalog table references with common table expressions.
updatedStatement =
Expand Down Expand Up @@ -302,8 +306,10 @@ StatementResult bindAndExecute(Statement statement) {
} else {
resultSet = spannerConnection.analyzeUpdateStatement(statement, QueryAnalyzeMode.PLAN);
}
} else {
} else if (parsedStatement.isQuery() || parsedStatement.hasReturningClause()) {
resultSet = spannerConnection.analyzeQuery(statement, QueryAnalyzeMode.PLAN);
} else {
return NO_RESULT;
}
return new QueryResult(resultSet);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.cloud.spanner.Statement;
import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement;
import com.google.cloud.spanner.connection.StatementResult;
import com.google.cloud.spanner.connection.StatementResult.ResultType;
import com.google.cloud.spanner.pgadapter.ConnectionHandler;
import com.google.cloud.spanner.pgadapter.error.PGExceptionFactory;
import com.google.cloud.spanner.pgadapter.metadata.DescribeResult;
Expand Down Expand Up @@ -102,7 +103,12 @@ public Future<StatementResult> describeAsync(BackendConnection backendConnection
this.describeResult =
Futures.lazyTransform(
statementResultFuture,
result -> new DescribeResult(this.givenParameterDataTypes, result.getResultSet()));
result ->
new DescribeResult(
this.givenParameterDataTypes,
result.getResultType() == ResultType.RESULT_SET
? result.getResultSet()
: null));
this.described = true;
return statementResultFuture;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.cloud.spanner.connection.Connection;
import com.google.cloud.spanner.connection.PostgreSQLStatementParser;
import com.google.cloud.spanner.connection.StatementResult;
import com.google.cloud.spanner.connection.StatementResult.ClientSideStatementType;
import com.google.cloud.spanner.connection.StatementResult.ResultType;
import com.google.cloud.spanner.pgadapter.ConnectionHandler;
import com.google.cloud.spanner.pgadapter.error.PGException;
Expand Down Expand Up @@ -138,6 +139,9 @@ public void close() throws Exception {
/** @return True if this is a select statement, false otherwise. */
public boolean containsResultSet() {
return this.parsedStatement.isQuery()
|| (this.parsedStatement.getType() == StatementType.CLIENT_SIDE
&& this.parsedStatement.getClientSideStatementType()
== ClientSideStatementType.RUN_BATCH)
|| (this.parsedStatement.isUpdate() && this.parsedStatement.hasReturningClause());
}

Expand Down Expand Up @@ -169,7 +173,8 @@ public long getUpdateCount(ResultNotReadyBehavior resultNotReadyBehavior) {
case QUERY:
return -1L;
case UPDATE:
return this.statementResult.getUpdateCount();
long res = this.statementResult.getUpdateCount();
return Math.max(res, 0L);
case CLIENT_SIDE:
case DDL:
case UNKNOWN:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,43 @@ public void testAutoDescribedStatementsAreReused() throws SQLException {
}
}

@Test
public void testDescribeDdlStatement() throws SQLException {
try (Connection connection = DriverManager.getConnection(createUrl())) {
try (PreparedStatement preparedStatement =
connection.prepareStatement("create table foo (id bigint primary key, value varchar)")) {
ParameterMetaData parameterMetaData = preparedStatement.getParameterMetaData();
assertEquals(0, parameterMetaData.getParameterCount());
assertNull(preparedStatement.getMetaData());
}
}
}

@Test
public void testDescribeClientSideNoResultStatement() throws SQLException {
try (Connection connection = DriverManager.getConnection(createUrl())) {
try (PreparedStatement preparedStatement = connection.prepareStatement("start batch dml")) {
ParameterMetaData parameterMetaData = preparedStatement.getParameterMetaData();
assertEquals(0, parameterMetaData.getParameterCount());
assertNull(preparedStatement.getMetaData());
}
}
}

@Test
public void testDescribeClientSideResultSetStatement() throws SQLException {
try (Connection connection = DriverManager.getConnection(createUrl())) {
try (PreparedStatement preparedStatement =
connection.prepareStatement("show statement_timeout")) {
SQLException exception =
assertThrows(SQLException.class, preparedStatement::getParameterMetaData);
assertEquals(
"ERROR: ResultSetMetadata are available only for results that were returned from Cloud Spanner",
exception.getMessage());
}
}
}

@Test
public void testMultipleQueriesInTransaction() throws SQLException {
String sql = "SELECT 1";
Expand Down

0 comments on commit cb616d8

Please sign in to comment.