Skip to content

fix: do not reserve statement name on prepare failure #4953

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.

Sign up for GitHub

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
olavloite merged 2 commits into from
Sep 28, 2026
Merged

fix: do not reserve statement name on prepare failure #4953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cbe03f3
fix: do not reserve statement name on prepare failure
olavloite Sep 24, 2026
b114fe3
fix: also refuse empty names for execute and deallocate
olavloite Sep 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/main/java/com/google/cloud/spanner/pgadapter/statements/DeallocateStatement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ static ParsedDeallocateStatement parse(String sql) {
"invalid prepared statement name", SQLState.InvalidSqlStatementName);
}
statementName = unquoteOrFoldIdentifier(name.name);
if (statementName == null || statementName.isEmpty()) {
throw PGExceptionFactory.newPGException(
"zero-length delimited identifier", SQLState.InvalidSqlStatementName);
}
}
parser.skipWhitespaces();
if (parser.getPos() < parser.getSql().length()) {
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/google/cloud/spanner/pgadapter/statements/ExecuteStatement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ static ParsedExecuteStatement parse(String sql) {
"invalid prepared statement name", SQLState.InvalidSqlStatementName);
}
String statementName = unquoteOrFoldIdentifier(name.name);
if (statementName == null || statementName.isEmpty()) {
throw PGExceptionFactory.newPGException(
"zero-length delimited identifier", SQLState.InvalidSqlStatementName);
}

List<String> parameters;
if (parser.eatToken("(")) {
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/com/google/cloud/spanner/pgadapter/statements/IntermediateStatement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,14 @@ public StatementType getStatementType() {
return this.parsedStatement.getType();
}

public ParsedStatement getParsedStatement() {
return this.parsedStatement;
}

public Statement getOriginalStatement() {
return this.originalStatement;
}

public String getSql() {
return this.originalStatement.getSql();
}
Expand Down
94 changes: 74 additions & 20 deletions src/main/java/com/google/cloud/spanner/pgadapter/statements/PrepareStatement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.cloud.spanner.pgadapter.statements;

import static com.google.cloud.spanner.pgadapter.statements.SimpleParser.unquoteOrFoldIdentifier;

import com.google.api.core.InternalApi;
import com.google.cloud.spanner.Dialect;
import com.google.cloud.spanner.Statement;
Expand All @@ -22,20 +24,21 @@
import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType;
import com.google.cloud.spanner.connection.StatementResult;
import com.google.cloud.spanner.pgadapter.ConnectionHandler;
import com.google.cloud.spanner.pgadapter.error.PGException;
import com.google.cloud.spanner.pgadapter.error.PGExceptionFactory;
import com.google.cloud.spanner.pgadapter.error.SQLState;
import com.google.cloud.spanner.pgadapter.metadata.OptionsMetadata;
import com.google.cloud.spanner.pgadapter.statements.BackendConnection.NoResult;
import com.google.cloud.spanner.pgadapter.statements.SimpleParser.TableOrIndexName;
import com.google.cloud.spanner.pgadapter.statements.SimpleParser.TypeDefinition;
import com.google.cloud.spanner.pgadapter.wireprotocol.ControlMessage.ManuallyCreatedToken;
import com.google.cloud.spanner.pgadapter.wireprotocol.ControlMessage.PreparedType;
import com.google.cloud.spanner.pgadapter.wireprotocol.DescribeMessage;
import com.google.cloud.spanner.pgadapter.wireprotocol.ParseMessage;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import java.util.List;
import java.util.concurrent.Future;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -144,28 +147,74 @@ public StatementType getStatementType() {
public void executeAsync(BackendConnection backendConnection) {
if (!this.executed) {
this.executed = true;
if (this.connectionHandler.hasStatement(this.preparedStatement.name)) {
PGException exception =
PGExceptionFactory.newPGException(
String.format(
"prepared statement \"%s\" already exists", this.preparedStatement.name),
SQLState.DuplicatePreparedStatement);
setFutureStatementResult(Futures.immediateFailedFuture(exception));
backendConnection.execute(
new InvalidStatement(
this.connectionHandler,
this.options,
this.parsedStatement,
this.originalStatement,
exception));
return;
}
IntermediatePreparedStatement statement =
ParseMessage.createStatement(
this.connectionHandler,
this.preparedStatement.name,
this.preparedStatement.parsedPreparedStatement,
this.preparedStatement.originalPreparedStatement,
this.preparedStatement.dataTypes);
if (statement instanceof InvalidStatement) {
setFutureStatementResult(Futures.immediateFailedFuture(statement.getException()));
backendConnection.execute((InvalidStatement) statement);
return;
}
ListenableFuture<StatementResult> analyzeResult;
try {
new ParseMessage(
connectionHandler,
preparedStatement.name,
preparedStatement.dataTypes,
preparedStatement.parsedPreparedStatement,
preparedStatement.originalPreparedStatement)
.send();
new DescribeMessage(
connectionHandler,
PreparedType.Statement,
preparedStatement.name,
ManuallyCreatedToken.MANUALLY_CREATED_TOKEN)
.send();
Future<StatementResult> describeFuture = statement.describeAsync(backendConnection);
Preconditions.checkState(
describeFuture instanceof ListenableFuture,
"describeAsync must return an instance of ListenableFuture");
analyzeResult = (ListenableFuture<StatementResult>) describeFuture;
} catch (Exception exception) {
setFutureStatementResult(Futures.immediateFailedFuture(exception));
PGException pgException = PGExceptionFactory.toPGException(exception);
setFutureStatementResult(Futures.immediateFailedFuture(pgException));
backendConnection.execute(
new InvalidStatement(
connectionHandler, options, parsedStatement, originalStatement, exception));
this.connectionHandler,
this.options,
this.parsedStatement,
this.originalStatement,
pgException));
return;
}
setFutureStatementResult(Futures.immediateFuture(new NoResult(getCommandTag())));
this.connectionHandler.registerStatement(this.preparedStatement.name, statement);
Futures.addCallback(
analyzeResult,
new FutureCallback<StatementResult>() {
@Override
public void onSuccess(StatementResult result) {}

@Override
public void onFailure(Throwable throwable) {
if (connectionHandler.hasStatement(preparedStatement.name)
&& connectionHandler.getStatement(preparedStatement.name) == statement) {
connectionHandler.closeStatement(preparedStatement.name);
}
}
},
MoreExecutors.directExecutor());
setFutureStatementResult(
Futures.transform(
analyzeResult,
ignored -> new NoResult(getCommandTag()),
MoreExecutors.directExecutor()));
}
}

Expand Down Expand Up @@ -196,6 +245,11 @@ static ParsedPreparedStatement parse(String sql) {
throw PGExceptionFactory.newPGException(
"invalid prepared statement name", SQLState.InvalidSqlStatementName);
}
String statementName = unquoteOrFoldIdentifier(name.name);
if (statementName == null || statementName.isEmpty()) {
Comment thread
olavloite marked this conversation as resolved.
Comment thread
olavloite marked this conversation as resolved.
throw PGExceptionFactory.newPGException(
"zero-length delimited identifier", SQLState.InvalidSqlStatementName);
}
ImmutableList.Builder<Integer> dataTypesBuilder = ImmutableList.builder();
if (parser.eatToken("(")) {
List<String> dataTypesNames = parser.parseExpressionList();
Expand All @@ -216,7 +270,7 @@ static ParsedPreparedStatement parse(String sql) {
"missing 'AS' keyword in PREPARE statement: " + sql, SQLState.SyntaxError);
}
return new ParsedPreparedStatement(
name.name,
statementName,
dataTypesBuilder.build().stream().mapToInt(i -> i).toArray(),
parser.getSql().substring(parser.getPos()).trim());
}
Expand Down
25 changes: 21 additions & 4 deletions src/main/java/com/google/cloud/spanner/pgadapter/wireprotocol/ParseMessage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement;
import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType;
import com.google.cloud.spanner.pgadapter.ConnectionHandler;
import com.google.cloud.spanner.pgadapter.error.PGExceptionFactory;
import com.google.cloud.spanner.pgadapter.error.SQLState;
import com.google.cloud.spanner.pgadapter.statements.BackendConnection;
import com.google.cloud.spanner.pgadapter.statements.CloseStatement;
import com.google.cloud.spanner.pgadapter.statements.CopyStatement;
Expand Down Expand Up @@ -74,7 +76,7 @@ public class ParseMessage extends AbstractQueryProtocolMessage {
protected static final char IDENTIFIER = 'P';

private final String name;
private final IntermediatePreparedStatement statement;
private IntermediatePreparedStatement statement;
private final int[] parameterDataTypes;

public ParseMessage(ConnectionHandler connection) throws Exception {
Expand Down Expand Up @@ -117,7 +119,7 @@ public ParseMessage(
createStatement(connection, name, parsedStatement, originalStatement, parameterDataTypes);
}

static IntermediatePreparedStatement createStatement(
public static IntermediatePreparedStatement createStatement(
ConnectionHandler connectionHandler,
String name,
ParsedStatement parsedStatement,
Expand Down Expand Up @@ -276,10 +278,25 @@ static IntermediatePreparedStatement createStatement(
@Override
void buffer(BackendConnection backendConnection) {
if (!Strings.isNullOrEmpty(this.name) && this.connection.hasStatement(this.name)) {
throw new IllegalStateException("Must close statement before reusing name.");
this.statement =
new InvalidStatement(
this.connection,
this.connection.getServer().getOptions(),
this.name,
this.statement.getParsedStatement(),
this.statement.getOriginalStatement(),
PGExceptionFactory.newPGException(
String.format("prepared statement \"%s\" already exists", this.name),
SQLState.DuplicatePreparedStatement));
if (backendConnection != null) {
backendConnection.execute((InvalidStatement) this.statement);
}
return;
}
if (this.statement instanceof InvalidStatement) {
backendConnection.execute((InvalidStatement) this.statement);
if (backendConnection != null) {
backendConnection.execute((InvalidStatement) this.statement);
}
}
this.connection.registerStatement(this.name, this.statement);
}
Expand Down
Loading
Loading