Skip to content
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

feat: handle implicit transactions and errors in batches #127

Merged
2 changes: 1 addition & 1 deletion .ci/run-with-credentials.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ e2e-psql)
echo "Starting PGAdapter"
ls -lh target
UBER_JAR="pgadapter.jar"
(java -jar target/"${UBER_JAR}" -p "${GOOGLE_CLOUD_PROJECT}" -i "${GOOGLE_CLOUD_INSTANCE}" -d "${GOOGLE_CLOUD_DATABASE_WITH_VERSION}" -e "${GOOGLE_CLOUD_ENDPOINT}" -s 4242 -q > /dev/null 2>&1) &
(java -jar target/"${UBER_JAR}" -p "${GOOGLE_CLOUD_PROJECT}" -i "${GOOGLE_CLOUD_INSTANCE}" -d "${GOOGLE_CLOUD_DATABASE_WITH_VERSION}" -e "${GOOGLE_CLOUD_ENDPOINT}" -s 4242 -q -ddl AutocommitImplicitTransaction > /dev/null 2>&1) &
BACK_PID=$!
sleep 1
# execute psql and evaluate result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@

/** Metadata extractor for CLI. */
public class OptionsMetadata {
public enum DdlTransactionMode {

// Disables all DDL batching and DDL statements in transactions. Only single DDL statements
// outside batches and transactions are allowed.
Single,

// Allows batches that contain only DDL statements outside explicit transactions.
// Does not allow mixing of DML, DQL and DDL statements in one batch.
// This is the default mode.
Batch,

// DDL statements automatically commit the active implicit transaction.
// Allows batches that contain mixes of DML, DQL and DDL statements.
// Fails if a DDL statement is executed in an explicit transaction.
AutocommitImplicitTransaction,

// DDL statements automatically commit the active explicit or implicit transaction.
// Allows batches that contain mixes of DML, DQL and DDL statements.
// Explicit transaction blocks that contain DDL statements will not be atomic.
AutocommitExplicitTransaction,
}

private static final Logger logger = Logger.getLogger(OptionsMetadata.class.getName());
private static final String DEFAULT_SERVER_VERSION = "1.0.0";
Expand All @@ -47,6 +68,7 @@ public class OptionsMetadata {
private static final String OPTION_BINARY_FORMAT = "b";
private static final String OPTION_AUTHENTICATE = "a";
private static final String OPTION_PSQL_MODE = "q";
private static final String OPTION_DDL_TRANSACTION_MODE = "ddl";
private static final String OPTION_JDBC_MODE = "jdbc";
private static final String OPTION_COMMAND_METADATA_FILE = "j";
private static final String OPTION_DISABLE_LOCALHOST_CHECK = "x";
Expand All @@ -69,6 +91,7 @@ public class OptionsMetadata {
private final boolean binaryFormat;
private final boolean authenticate;
private final boolean requiresMatcher;
private final DdlTransactionMode ddlTransactionMode;
private final boolean replaceJdbcMetadataQueries;
private final boolean disableLocalhostCheck;
private final JSONObject commandMetadataJSON;
Expand All @@ -92,6 +115,8 @@ public OptionsMetadata(String[] args) {
this.requiresMatcher =
commandLine.hasOption(OPTION_PSQL_MODE)
|| commandLine.hasOption(OPTION_COMMAND_METADATA_FILE);
this.ddlTransactionMode =
parseDdlTransactionMode(commandLine.getOptionValue(OPTION_DDL_TRANSACTION_MODE));
this.replaceJdbcMetadataQueries = commandLine.hasOption(OPTION_JDBC_MODE);
this.commandMetadataJSON = buildCommandMetadataJSON(commandLine);
this.propertyMap = parseProperties(commandLine.getOptionValue(OPTION_JDBC_PROPERTIES, ""));
Expand All @@ -117,6 +142,7 @@ public OptionsMetadata(
this.binaryFormat = forceBinary;
this.authenticate = authenticate;
this.requiresMatcher = requiresMatcher;
this.ddlTransactionMode = DdlTransactionMode.AutocommitImplicitTransaction;
this.replaceJdbcMetadataQueries = replaceJdbcMetadataQueries;
this.commandMetadataJSON = commandMetadata;
this.propertyMap = new HashMap<>();
Expand All @@ -141,6 +167,19 @@ private Map<String, String> parseProperties(String propertyOptions) {
return properties;
}

private DdlTransactionMode parseDdlTransactionMode(String value) {
if (value == null) {
return DdlTransactionMode.Batch;
}
try {
return DdlTransactionMode.valueOf(value);
} catch (IllegalArgumentException e) {
// Catch and rethrow to give a better error message.
throw new IllegalArgumentException(
String.format("Invalid ddl-batching mode value specified: %s", value));
}
}

/**
* Takes the proxy port option result and parses it accordingly to fit port specs.
*
Expand Down Expand Up @@ -307,6 +346,26 @@ private CommandLine buildOptions(String[] args) {
+ " added performance cost. PSQL mode is implemented using predefined dynamic matchers"
+ " and as such cannot be used with the option -j. This mode should not be used for"
+ " production, and we do not guarantee its functionality beyond the basics.");
options.addOption(
OPTION_DDL_TRANSACTION_MODE,
"ddl-transaction-mode",
true,
String.format(
"Sets the way PGAdapter should handle DDL statements in implicit and explicit "
+ "transactions. Cloud Spanner does not support DDL transactions. The possible modes "
+ "are:\n"
+ "%s: Only allows single DDL statements outside implicit and explicit transactions.\n"
+ "%s: Allows batches that contain only DDL statements. "
+ "Does not allow mixed batches of DDL and other statements, or DDL statements in transactions.\n"
+ "%s: Allows mixed batches of DDL and other statements. "
+ "Automatically commits the implicit transaction when a DDL statement is encountered in a batch. "
+ "DDL statements in explicit transactions are not allowed.\n"
+ "%s: Allows mixed batches of DDL and other statements. "
+ "Automatically commits the current transaction when a DDL statement is encountered.",
DdlTransactionMode.Single,
DdlTransactionMode.Batch,
DdlTransactionMode.AutocommitImplicitTransaction,
DdlTransactionMode.AutocommitExplicitTransaction));
options.addOption(
OPTION_JDBC_MODE,
"jdbc-mode",
Expand Down Expand Up @@ -372,6 +431,7 @@ private CommandLine buildOptions(String[] args) {

CommandLineParser parser = new DefaultParser();
HelpFormatter help = new HelpFormatter();
help.setWidth(120);
try {
CommandLine commandLine = parser.parse(options, args);
if (commandLine.hasOption(OPTION_HELP)) {
Expand All @@ -385,6 +445,10 @@ private CommandLine buildOptions(String[] args) {
}
}

public DdlTransactionMode getDdlTransactionMode() {
return this.ddlTransactionMode;
}

public boolean isBinaryFormat() {
return this.binaryFormat;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ public void handleExecutionException(int index, SpannerException e) {

@Override
public void execute() {
this.executed = true;
this.executedCount++;
try {
parseCopyStatement();
queryInformationSchema();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ public void setParameterDataTypes(int[] parameterDataTypes) {

@Override
public void execute() {
this.executed = true;
// If the portal has already been described, the statement has already been executed, and we
// don't need to do that once more.
if (getStatementResult(0) == null) {
this.executedCount++;
try {
if (!connection.isInTransaction()
// TODO(230579451): Refactor to use ClientSideStatement information.
Expand Down
Loading