Hi all,
The behavior of rollback when an error occurs on an handle is
controlled by extending Protocol with "$PROTNUM-[0|1|2]" where:
- 0 = let the application handle rollbacks
- 1 = rollback whole transaction when an error occurs
- 2 = rollback only statement that failed
Using such an extension is somewhat awkward as a single string is used
for two settings... The proposed attached patch adds a new parameter
called RollbackError that allows to control the behavior of rollback
on error with a different parameter.
For backward-compatibility purposes, this patch does not break the old
grammar of Protocol: it just gives the priority to RollbackError if
both Protocol and RollbackError are set for a connection. Regression
tests to test RollbackError and combinations of RollbackError/Protocol
are added in the patch in the existing test error-rollback (which has
needed some refactoring, older tests are not impacted). Docs are
included as well.
I thought first about including that in my cleanup work for 9.4, but
as this actually does not break the driver it may be worth adding it
directly to master, explaining the patch attached here. Comments
welcome. Note that if there are objections I do not mind adding that
for the work that would be merged later to 9.4 builds.
Regards,
--
Michael
diff --git a/connection.c b/connection.c
index 8601cb7..9407619 100644
--- a/connection.c
+++ b/connection.c
@@ -288,7 +288,8 @@ CC_conninfo_init(ConnInfo *conninfo, UInt4 option)
conninfo->bytea_as_longvarbinary = -1;
conninfo->use_server_side_prepare = -1;
conninfo->lower_case_identifier = -1;
- conninfo->rollback_on_error = -1;
+ conninfo->rollback_error_value = -1;
+ conninfo->is_rollback_error = -1;
conninfo->force_abbrev_connstr = -1;
conninfo->bde_environment = -1;
conninfo->fake_mss = -1;
@@ -341,7 +342,8 @@ CC_copy_conninfo(ConnInfo *ci, const ConnInfo *sci)
CORR_VALCPY(bytea_as_longvarbinary);
CORR_VALCPY(use_server_side_prepare);
CORR_VALCPY(lower_case_identifier);
- CORR_VALCPY(rollback_on_error);
+ CORR_VALCPY(rollback_error_value);
+ CORR_VALCPY(is_rollback_error);
CORR_VALCPY(force_abbrev_connstr);
CORR_VALCPY(bde_environment);
CORR_VALCPY(fake_mss);
@@ -2733,7 +2735,7 @@ CC_send_query_append(ConnectionClass *self, const char
*query, QueryInfo *qi, UD
BOOL ignore_abort_on_conn = ((flag & IGNORE_ABORT_ON_CONN) != 0),
create_keyset = ((flag & CREATE_KEYSET) != 0),
issue_begin = ((flag & GO_INTO_TRANSACTION) != 0 &&
!CC_is_in_trans(self)),
- rollback_on_error, query_rollback, end_with_commit;
+ rollback_error_value, query_rollback, end_with_commit;
const char *wq;
char swallow, *ptr;
@@ -2844,13 +2846,13 @@ CC_send_query_append(ConnectionClass *self, const char
*query, QueryInfo *qi, UD
return res;
}
- rollback_on_error = (flag & ROLLBACK_ON_ERROR) != 0;
+ rollback_error_value = (flag & ROLLBACK_ON_ERROR) != 0;
end_with_commit = (flag & END_WITH_COMMIT) != 0;
#define return DONT_CALL_RETURN_FROM_HERE???
consider_rollback = (issue_begin || (CC_is_in_trans(self) &&
!CC_is_in_error_trans(self)) || strnicmp(query, "begin", 5) == 0);
- if (rollback_on_error)
- rollback_on_error = consider_rollback;
- query_rollback = (rollback_on_error && !end_with_commit &&
PG_VERSION_GE(self, 8.0));
+ if (rollback_error_value)
+ rollback_error_value = consider_rollback;
+ query_rollback = (rollback_error_value && !end_with_commit &&
PG_VERSION_GE(self, 8.0));
if (!query_rollback && consider_rollback && !end_with_commit)
{
if (stmt)
@@ -3341,7 +3343,7 @@ cleanup:
CC_on_abort(self, CONN_DEAD);
retres = NULL;
}
- if (rollback_on_error && CC_is_in_trans(self) &&
!discard_next_savepoint)
+ if (rollback_error_value && CC_is_in_trans(self) &&
!discard_next_savepoint)
{
char cmd[64];
diff --git a/connection.h b/connection.h
index e92ff61..453ed5b 100644
--- a/connection.h
+++ b/connection.h
@@ -287,7 +287,16 @@ typedef struct
char database[MEDIUM_REGISTRY_LEN];
char username[MEDIUM_REGISTRY_LEN];
pgNAME password;
+
+ /*
+ * Protocol number, this can be used as well to define the behavior
+ * of rollback in case of error.
+ */
char protocol[SMALL_REGISTRY_LEN];
+
+ /* Value of RollbackError */
+ char rollback_error[SMALL_REGISTRY_LEN];
+
char port[SMALL_REGISTRY_LEN];
char sslmode[16];
char onlyread[SMALL_REGISTRY_LEN];
@@ -308,7 +317,19 @@ typedef struct
signed char bytea_as_longvarbinary;
signed char use_server_side_prepare;
signed char lower_case_identifier;
- signed char rollback_on_error;
+
+ /*
+ * Control behavior of rollback on error.
+ *
+ * rollback_error_value is used to store the value given by either
+ * Protovol (as an extension of the protocol number) or RollbackError.
+ * is_rollback_error is set to true if the value set is obtained with
+ * RollbackError and not as an extension of Protocol. Priority is given
+ * to RollbackError if both Protocol and RollbackError are set.
+ */
+ signed char rollback_error_value;
+ signed char is_rollback_error;
+
signed char force_abbrev_connstr;
signed char bde_environment;
signed char fake_mss;
diff --git a/dlg_specific.c b/dlg_specific.c
index 8e051af..ff6066b 100644
--- a/dlg_specific.c
+++ b/dlg_specific.c
@@ -229,16 +229,11 @@ inolog("force_abbrev=%d abbrev=%d\n",
ci->force_abbrev_connstr, abbrev);
inolog("hlen=%d", hlen);
if (!abbrev)
{
- char protocol_and[16];
-
- if (ci->rollback_on_error >= 0)
- snprintf(protocol_and, sizeof(protocol_and), "%s-%d",
ci->protocol, ci->rollback_on_error);
- else
- strncpy_null(protocol_and, ci->protocol,
sizeof(protocol_and));
olen = snprintf(&connect_string[hlen], nlen, ";"
INI_SSLMODE "=%s;"
INI_READONLY "=%s;"
INI_PROTOCOL "=%s;"
+ INI_ROLLBACK_ON_ERROR "=%d;"
INI_FAKEOIDINDEX "=%s;"
INI_SHOWOIDCOLUMN "=%s;"
INI_ROWVERSIONING "=%s;"
@@ -276,7 +271,8 @@ inolog("hlen=%d", hlen);
#endif /* _HANDLE_ENLIST_IN_DTC_ */
,ci->sslmode
,ci->onlyread
- ,protocol_and
+ ,ci->protocol
+ ,ci->rollback_error_value
,ci->fake_oid_index
,ci->show_oid_column
,ci->row_versioning
@@ -408,22 +404,22 @@ inolog("hlen=%d", hlen);
ci->int8_as,
ci->drivers.extra_systable_prefixes,
EFFECTIVE_BIT_COUNT, flag);
- if (olen < nlen && (PROTOCOL_74(ci) || ci->rollback_on_error >=
0))
+ if (olen < nlen && (PROTOCOL_74(ci) || ci->rollback_error_value
>= 0))
{
hlen = strlen(connect_string);
nlen = MAX_CONNECT_STRING - hlen;
/*
* The PROTOCOL setting must be placed after CX flag
* so that this option can override the CX setting.
+ * Complete it with RollbackError.
*/
- if (ci->rollback_on_error >= 0)
+ if (ci->rollback_error_value >= 0)
olen = snprintf(&connect_string[hlen], nlen, ";"
- ABBR_PROTOCOL "=%s-%d",
- ci->protocol, ci->rollback_on_error);
+ ABBR_PROTOCOL "=%s;"
ABBR_ROLLBACK_ON_ERROR "=%d",
+ ci->protocol, ci->rollback_error_value);
else
olen = snprintf(&connect_string[hlen], nlen, ";"
- ABBR_PROTOCOL "=%s",
- ci->protocol);
+ ABBR_PROTOCOL "=%s", ci->protocol);
}
}
if (olen < nlen)
@@ -541,22 +537,33 @@ copyAttributes(ConnInfo *ci, const char *attribute, const
char *value)
else if (stricmp(attribute, INI_PROTOCOL) == 0 || stricmp(attribute,
ABBR_PROTOCOL) == 0)
{
char *ptr;
-
ptr = strchr(value, '-');
- if (ptr)
+
+ /*
+ * Copy value controlling rollback on error if an extension of
+ * Protocol is found and if it is not set already by
RollbackError.
+ */
+ if (ptr && ci->is_rollback_error < 0)
{
if ('-' != *value)
{
*ptr = '\0';
strcpy(ci->protocol, value);
}
- ci->rollback_on_error = atoi(ptr + 1);
- mylog("rollback_on_error=%d\n", ci->rollback_on_error);
+ ci->rollback_error_value = atoi(ptr + 1);
+ mylog("rollback_error_value=%d\n",
ci->rollback_error_value);
}
else
strcpy(ci->protocol, value);
}
+ else if (stricmp(attribute, INI_ROLLBACK_ON_ERROR) == 0 ||
+ stricmp(attribute, ABBR_ROLLBACK_ON_ERROR) == 0)
+ {
+ ci->rollback_error_value = atoi(value);
+ ci->is_rollback_error = 0;
+ }
+
else if (stricmp(attribute, INI_SHOWOIDCOLUMN) == 0 ||
stricmp(attribute, ABBR_SHOWOIDCOLUMN) == 0)
strcpy(ci->show_oid_column, value);
@@ -663,7 +670,7 @@ copyAttributes(ConnInfo *ci, const char *attribute, const
char *value)
else
found = FALSE;
- mylog("%s:
DSN='%s',server='%s',dbase='%s',user='%s',passwd='%s',port='%s',onlyread='%s',protocol='%s',conn_settings='%s',disallow_premature=%d)\n",
func, ci->dsn, ci->server, ci->database, ci->username,
NAME_IS_VALID(ci->password) ? "xxxxx" : "", ci->port, ci->onlyread,
ci->protocol, ci->conn_settings, ci->disallow_premature);
+ mylog("%s:
DSN='%s',server='%s',dbase='%s',user='%s',passwd='%s',port='%s',onlyread='%s',protocol='%s',rollback_error='%s',conn_settings='%s',disallow_premature=%d)\n",
func, ci->dsn, ci->server, ci->database, ci->username,
NAME_IS_VALID(ci->password) ? "xxxxx" : "", ci->port, ci->onlyread,
ci->protocol, ci->rollback_error, ci->conn_settings, ci->disallow_premature);
return found;
}
@@ -893,21 +900,45 @@ getDSNinfo(ConnInfo *ci, char overwrite)
if (ci->show_system_tables[0] == '\0' || overwrite)
SQLGetPrivateProfileString(DSN, INI_SHOWSYSTEMTABLES, "",
ci->show_system_tables, sizeof(ci->show_system_tables), ODBC_INI);
+ /*
+ * Set up value for Protocol. Error on rollback
+ */
if (ci->protocol[0] == '\0' || overwrite)
{
char *ptr;
SQLGetPrivateProfileString(DSN, INI_PROTOCOL, "", ci->protocol,
sizeof(ci->protocol), ODBC_INI);
- if (ptr = strchr(ci->protocol, '-'), NULL != ptr)
+
+ /*
+ * Check if behavior of rollback on error is set as an extension
+ * of Protocol. Note that if this parameter has already been set
+ * by RollbackError, we simply ignore the value set here.
+ */
+ if (ptr = strchr(ci->protocol, '-'), NULL != ptr &&
+ ci->is_rollback_error < 0)
{
*ptr = '\0';
- if (overwrite || ci->rollback_on_error < 0)
+ if (overwrite || ci->rollback_error_value < 0)
{
- ci->rollback_on_error = atoi(ptr + 1);
- mylog("rollback_on_error=%d\n",
ci->rollback_on_error);
+ ci->rollback_error_value = atoi(ptr + 1);
+ mylog("rollback_error_value=%d\n",
ci->rollback_error_value);
}
}
}
+ /*
+ * Set up value for RollbackError. This gets priority over Protocol.
+ */
+ if (ci->rollback_error[0] == '\0' || overwrite)
+ {
+ if ((ci->rollback_error_value < 0 && ci->is_rollback_error < 0)
+ || overwrite)
+ {
+ ci->rollback_error_value = atoi(ci->rollback_error);
+ ci->is_rollback_error = 0;
+ mylog("rollback_error_value=%d\n",
ci->rollback_error_value);
+ }
+ }
+
if (NAME_IS_NULL(ci->conn_settings) || overwrite)
{
SQLGetPrivateProfileString(DSN, INI_CONNSETTINGS, "",
encoded_item, sizeof(encoded_item), ODBC_INI);
@@ -1205,12 +1236,14 @@ writeDSNinfo(const ConnInfo *ci)
ci->show_system_tables,
ODBC_INI);
- if (ci->rollback_on_error >= 0)
- sprintf(temp, "%s-%d", ci->protocol, ci->rollback_on_error);
- else
- strncpy_null(temp, ci->protocol, sizeof(temp));
SQLWritePrivateProfileString(DSN,
INI_PROTOCOL,
+ ci->protocol,
+ ODBC_INI);
+
+ sprintf(temp, "%d", ci->rollback_error_value);
+ SQLWritePrivateProfileString(DSN,
+
INI_ROLLBACK_ON_ERROR,
temp,
ODBC_INI);
diff --git a/dlg_specific.h b/dlg_specific.h
index c73bb64..d2d1575 100644
--- a/dlg_specific.h
+++ b/dlg_specific.h
@@ -75,6 +75,8 @@ extern "C" {
#define ABBR_COMMLOG "B3"
#define INI_PROTOCOL "Protocol" /* What protocol (6.2)
*/
#define ABBR_PROTOCOL "A1"
+#define INI_ROLLBACK_ON_ERROR "RollbackError" /* Rollback on error */
+#define ABBR_ROLLBACK_ON_ERROR "AA"
#define INI_OPTIMIZER "Optimizer" /* Use backend genetic
* optimizer */
#define ABBR_OPTIMIZER "B4"
diff --git a/docs/config-opt.html b/docs/config-opt.html
index 6673bc2..bba3266 100644
--- a/docs/config-opt.html
+++ b/docs/config-opt.html
@@ -146,6 +146,17 @@
</TR>
<TR>
<TD WIDTH=38%>
+ Rollback on error
+ </TD>
+ <TD WIDTH=31%>
+ RollbackError
+ </TD>
+ <TD WIDTH=31%>
+ AA
+ </TD>
+ </TR>
+ <TR>
+ <TD WIDTH=38%>
Backend enetic optimizer
</TD>
<TD WIDTH=31%>
diff --git a/docs/config.html b/docs/config.html
index cb1c326..b0b727a 100644
--- a/docs/config.html
+++ b/docs/config.html
@@ -238,23 +238,31 @@ with 6.4 and higher backends.<br /> </li>
with 7.4 and higher backends.<br /> </li>
</ul></li>
-<li><b>Level of rollback on errors:</b> Specifies what to rollback should an
-error occur.<br />
+<li><b>RollbackError</b>: Level of rollback on errors, specifies what to
+rollback should an error occur.<br />
<ul>
<li><i>Nop(0):</i> Don't rollback anything and let the application handle the
-error.<br /> </li>
-
-<li><i>Transaction(1):</i> Rollback the entire transaction.<br /> </li>
-
-<li><i>Statement(2):</i> Rollback the statement.<br /> </li>
-<br>
-<b>Notes in a setup: This specification is set up with a PROTOCOL option
parameter.</b><br><br>
-PROTOCOL=[6.2|6.3|6.4|7.4][-(0|1|2)]<br>
-default value is a sentence unit (it is a transaction unit before 8.0).<br>
-<br>
-
-</ul></li>
+error.<br /></li>
+
+<li><i>Transaction(1):</i> Rollback the entire transaction.<br /></li>
+
+<li><i>Statement(2):</i> Rollback the statement.<br /></li>
+<b>Notes:</b>
+ <ul>
+ <li>
+ This parameter can be set as an extension of PROTOCOL option
+ parameter.<br />
+ PROTOCOL=[6.2|6.3|6.4|7.4][-(0|1|2)]<br />
+ Default value is a sentence unit (it is a transaction unit
+ before 8.0).<br />
+ </li>
+ <li>
+ This parameter can be set with ROLLBACKERROR as well. This takes
+ precedence on PROTOCOL.<bt />
+ </li>
+ </ul>
+</ul>.<br /> </li>
<li><b>OID Options:</b><br />
diff --git a/execute.c b/execute.c
index 1cc4f04..d502df2 100644
--- a/execute.c
+++ b/execute.c
@@ -652,7 +652,7 @@ inolog("%s:%p->internal=%d\n", func, stmt, stmt->internal);
if (conn)
ci = &conn->connInfo;
ret = 0;
- if (!ci || ci->rollback_on_error < 0) /* default */
+ if (!ci || ci->rollback_error_value < 0) /* default */
{
if (conn && PG_VERSION_GE(conn, 8.0))
ret = 2; /* statement rollback */
@@ -661,7 +661,7 @@ inolog("%s:%p->internal=%d\n", func, stmt, stmt->internal);
}
else
{
- ret = ci->rollback_on_error;
+ ret = ci->rollback_error_value;
if (2 == ret && PG_VERSION_LT(conn, 8.0))
ret = 1;
}
diff --git a/test/expected/error-rollback.out b/test/expected/error-rollback.out
index 436a328..da9cdf3 100644
--- a/test/expected/error-rollback.out
+++ b/test/expected/error-rollback.out
@@ -1,5 +1,5 @@
\! ./src/error-rollback-test
-Test for rollback protocol 0
+Test for Protocol=7.4-0
connected
Executing query that will succeed
Executing query that will fail
@@ -10,7 +10,7 @@ Executing query that will succeed
Result set:
1
disconnecting
-Test for rollback protocol 1
+Test for Protocol=7.4-1
connected
Executing query that will succeed
Executing query that will fail
@@ -20,7 +20,71 @@ Executing query that will succeed
Result set:
1
disconnecting
-Test for rollback protocol 2
+Test for Protocol=7.4-2
+connected
+Executing query that will succeed
+Executing query that will fail
+Failed to execute statement
+22P02=ERROR: invalid input syntax for integer: "foo"
+Executing query that will succeed
+Result set:
+1
+1
+disconnecting
+Test for RollbackError=0
+connected
+Executing query that will succeed
+Executing query that will fail
+Failed to execute statement
+22P02=ERROR: invalid input syntax for integer: "foo"
+Rolling back with SQLEndTran
+Executing query that will succeed
+Result set:
+1
+disconnecting
+Test for RollbackError=1
+connected
+Executing query that will succeed
+Executing query that will fail
+Failed to execute statement
+22P02=ERROR: invalid input syntax for integer: "foo"
+Executing query that will succeed
+Result set:
+1
+disconnecting
+Test for RollbackError=2
+connected
+Executing query that will succeed
+Executing query that will fail
+Failed to execute statement
+22P02=ERROR: invalid input syntax for integer: "foo"
+Executing query that will succeed
+Result set:
+1
+1
+disconnecting
+Test for RollbackError=0;Protocol=7.4-2
+connected
+Executing query that will succeed
+Executing query that will fail
+Failed to execute statement
+22P02=ERROR: invalid input syntax for integer: "foo"
+Rolling back with SQLEndTran
+Executing query that will succeed
+Result set:
+1
+disconnecting
+Test for RollbackError=1;Protocol=7.4-0
+connected
+Executing query that will succeed
+Executing query that will fail
+Failed to execute statement
+22P02=ERROR: invalid input syntax for integer: "foo"
+Executing query that will succeed
+Result set:
+1
+disconnecting
+Test for RollbackError=2;Protocol=7.4-1
connected
Executing query that will succeed
Executing query that will fail
diff --git a/test/src/error-rollback-test.c b/test/src/error-rollback-test.c
index 01c0f0c..bbff007 100644
--- a/test/src/error-rollback-test.c
+++ b/test/src/error-rollback-test.c
@@ -142,18 +142,19 @@ error_rollback_print(void)
print_result(hstmt);
}
-int
-main(int argc, char **argv)
+/*
+ * Test for rollback protocol 0.
+ *
+ * Additional options can be specified to play with combinations of
+ * Protocol and RollbackError. With protocol 0, it is the responsability
+ * of application to issue rollbacks.
+ */
+void
+error_rollback_test_0(char *options)
{
SQLRETURN rc;
- /*
- * Test for protocol at 0.
- * Do nothing when error occurs and let application do necessary
- * ROLLBACK on error.
- */
- printf("Test for rollback protocol 0\n");
- error_rollback_init("Protocol=7.4-0");
+ error_rollback_init(options);
/* Insert a row correctly */
error_rollback_exec_success();
@@ -181,17 +182,25 @@ main(int argc, char **argv)
/* Clean up */
error_rollback_clean();
+}
- /*
- * Test for rollback protocol 1
- * In case of an error rollback the entire transaction.
- */
- printf("Test for rollback protocol 1\n");
- error_rollback_init("Protocol=7.4-1");
+/*
+ * Test for rollback protocols 1 or 2
+ *
+ * Options can be specified to manipulate the protocol used with Protocol
+ * and RollbackError. When priting results, there should be one row for
+ * protocol 1 as rollback is done for an entire transaction is case of
+ * failure. There will be two rows for protocol 2 as tollback is issued
+ * for each statement.
+ */
+void
+error_rollback_test_12(char *options)
+{
+ error_rollback_init(options);
/*
- * Insert a row, trigger an error, and re-insert a row. Only one
- * row should be visible here.
+ * Insert a row, trigger an error, and re-insert a row. Depending
+ * on the protocol, one or two rows should be visible.
*/
error_rollback_exec_success();
error_rollback_exec_failure();
@@ -200,23 +209,40 @@ main(int argc, char **argv)
/* Clean up */
error_rollback_clean();
+}
+int
+main(int argc, char **argv)
+{
/*
- * Test for rollback protocol 2
- * In the case of an error rollback only the latest statement.
+ * Test for Protocol only
*/
- printf("Test for rollback protocol 2\n");
- error_rollback_init("Protocol=7.4-2");
+ printf("Test for Protocol=7.4-0\n");
+ error_rollback_test_0("Protocol=7.4-0");
+ printf("Test for Protocol=7.4-1\n");
+ error_rollback_test_12("Protocol=7.4-1");
+ printf("Test for Protocol=7.4-2\n");
+ error_rollback_test_12("Protocol=7.4-2");
/*
- * Similarly to previous case, do insert, error and insert. This
- * time two rows should be visible.
+ * Now do the same tests as previously, but this time for
+ * RollbackError.
*/
- error_rollback_exec_success();
- error_rollback_exec_failure();
- error_rollback_exec_success();
- error_rollback_print();
+ printf("Test for RollbackError=0\n");
+ error_rollback_test_0("RollbackError=0");
+ printf("Test for RollbackError=1\n");
+ error_rollback_test_12("RollbackError=1");
+ printf("Test for RollbackError=2\n");
+ error_rollback_test_12("RollbackError=2");
- /* Clean up */
- error_rollback_clean();
+ /*
+ * Combinations of RollbackError and Protocol, the former
+ * should have the priority.
+ */
+ printf("Test for RollbackError=0;Protocol=7.4-2\n");
+ error_rollback_test_0("RollbackError=0;Protocol=7.4-2");
+ printf("Test for RollbackError=1;Protocol=7.4-0\n");
+ error_rollback_test_12("RollbackError=1;Protocol=7.4-0\n"); /* 1 row */
+ printf("Test for RollbackError=2;Protocol=7.4-1\n");
+ error_rollback_test_12("RollbackError=2;Protocol=7.4-1\n"); /* 2 rows */
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
