Don't issue a BEGIN when running VACUUM in auto-commit mode.

Normally in auto-commit mode the driver begins a new transaction
implicitly at the first statement, by sending a BEGIN statement. However,
some commands, like VACUUM, cannot be run in a transaction block, and you
will get an error like "VACUUM cannot run inside a transaction block" from
the server. In UseServerSidePrepare=0 mode, the code looks at the first word
of the query to determine if the statement is one of the special ones, and
if so, didn't begin a new transaction even when auto-commit mode is
disabled. However, in UseServerSidePrepare=1 mode, when using SQLPrepare/
SQLExecute to run the VACUUM, that check was not made. Fix that.

There was one more related inconsistency between UseServerSidePrepare modes.
Without server-side-prepares, if you issued an explicit BEGIN in auto-commit
mode, the implicit BEGIN was ont sent. But without server-side prepares, it
was. It seems best to send the implicit BEGIN in both cases, because then
you get a warning from the backend about the second BEGIN. That's a good
thing, because a sane ODBC application should be using the ODBC function
SQLEndTran() for transaction control, not explicit BEGIN/COMMIT.

Also add a test case for executing VACUUM, with and without autocommit.

1 files changed, 33 insertions, 11 deletions

diff --git a/statement.c b/statement.c
index 7af103f..ad98223 100644
--- a/statement.c
+++ b/statement.c
@@ -142,6 +142,14 @@ static const struct
 	,{
 		STMT_TYPE_EXPLAIN, "EXPLAIN"
 	}
+
+	/*
+	 * Special-commands that cannot be run in a transaction block. This isn't
+	 * as granular as it could be. VACUUM can never be run in a transaction
+	 * block, but some variants of REINDEX and CLUSTER can be. CHECKPOINT
+	 * doesn't throw an error if you do, but it cannot be rolled back so
+	 * there's no point in beginning a new transaction for it.
+	 */
 	,{
 		STMT_TYPE_SPECIAL, "VACUUM"
 	}
@@ -154,6 +162,7 @@ static const struct
 	,{
 		STMT_TYPE_SPECIAL, "CHECKPOINT"
 	}
+
 	,{
 		STMT_TYPE_WITH, "WITH"
 	}
@@ -1897,15 +1906,22 @@ SC_execute(StatementClass *self)
 	    /* || SC_is_with_hold(self) thiw would lose the performance */
 		 ))
 		issue_begin = FALSE;
-	else
+	else if (self->statement_type == STMT_TYPE_SPECIAL)
 	{
-		switch (self->statement_type)
-		{
-			case STMT_TYPE_START:
-			case STMT_TYPE_SPECIAL:
-				issue_begin = FALSE;
-				break;
-		}
+		/*
+		 * Some utility commands like VACUUM cannot be run in a transaction
+		 * block, so don't begin one even if auto-commit mode is disabled.
+		 *
+		 * An application should never issue an explicit BEGIN when
+		 * auto-commit mode is disabled (probably not even when it's enabled,
+		 * actually). We used to also suppress the implicit BEGIN when the
+		 * statement was of STMT_TYPE_START type, ie. if the application
+		 * issued an explicit BEGIN, but that actually seems like a bad idea.
+		 * First of all, if you issue a BEGIN twice the backend will give a
+		 * warning which can be helpful to spot mistakes in the application
+		 * (because it shouldn't be doing that).
+		 */
+		issue_begin = FALSE;
 	}
 	if (issue_begin)
 	{
@@ -2465,10 +2481,16 @@ RequestStart(StatementClass *stmt, ConnectionClass *conn, const char *func)
 		SC_set_error(stmt, STMT_INTERNAL_ERROR, emsg, func);
 		return FALSE;
 	}
-	if (!CC_is_in_trans(conn) && CC_loves_visible_trans(conn))
+
+	/*
+	 * In auto-commit mode, begin a new transaction implicitly if no
+	 * transaction is in progress yet. However, some special statements like
+	 * VACUUM and CLUSTER cannot be run in a transaction block.
+	 */
+	if (!CC_is_in_trans(conn) && CC_loves_visible_trans(conn) &&
+		stmt->statement_type != STMT_TYPE_SPECIAL)
 	{
-		if (ret = CC_begin(conn), !ret)
-			return ret;
+		ret = CC_begin(conn);
 	}
 	return ret;
 }