Explicit subtransactions for PL/Tcl
Collegues!
Recently I've found out that PL/Python have very nice feature - explicit
subtransaction object, which allows to execute block of code in the
context of subtransaction.
I've quite surprised that other PL languages, shipped with PostgreSQL do
not have such useful construction.
If it might require considerable trickery to add such functionality into
PL/Perl, Tcl allows to add new control stuctures very easily.
I'm attaching the patch which implements subtransaction command for
PL/Tcl which does almost same as PL/Python plpy.subtransction context
manager object does: executes a block of Tcl code in the context of
subtransaction, rolls subtransaction back if error occures and commits
it otherwise.
It looks like
subtransaction {
...some Tcl code...
}
Typically one would use it inside Tcl catch statement:
if [catch {
subtransaction {
spi_exec "insert into..."
...
}
} errormsg] {
# Handle an error
}
See documentation and tests included in the patch for more complete
examples.
Just like corresponding Python construction, this command doesn't
replace language builtin exception handling, just adds subtransaction
support to it.
Patch includes sufficiently less tests than python subtransaction tests,
because Tcl implementation is way simpler than python one, and doesn't
have syntactic variatons which depend on language version.
Also entering and exiting subtransactions are in the same piece of code
rather than in separate __enter__ and __exit__ methods as in Python, so
there is no possibility to call exit without enter or vice versa.
--
Victor Wagner <vitus@wagner.pp.ru>
Attachments:
tcl_subtransaction_01.patchtext/x-patchDownload
On Sun, 8 Jan 2017 20:57:50 +0300
Victor Wagner <vitus@wagner.pp.ru> wrote:
Collegues!
Recently I've found out that PL/Python have very nice feature -
explicit subtransaction object, which allows to execute block of code
in the context of subtransaction.
[skip]
I'm attaching the patch which implements subtransaction command for
Sorry, unfortunately attached empty file instead of patch
--
Victor Wagner <vitus@wagner.pp.ru>
Attachments:
tcl_subtransaction_01.patchtext/x-patchDownload
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index 8afaf4a..7a532b7 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,85 @@ if {[catch { spi_exec $sql_command }]} {
is a global variable.)
</para>
</sect1>
+ <sect1 id="pltcl-subransaction">
+ <title>Explicit Subtransactions</title>
+ <para>
+ Recovering from errors caused by database access as described in
+ <xref linkend="pltcl-error-handling"> can lead to an undesirable
+ situation where some operations succeed before one of them fails,
+ and after recovering from that error the data is left in an
+ inconsistent state. PL/Tcl offers a solution to this problem in
+ the form of explicit subtransactions.
+ </para>
+
+ <sect2>
+ <title>Subtransaction command</title>
+
+ <para>
+ Consider a function that implements a transfer between two
+ accounts:
+<programlisting>
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+ set result [format "error transferring funds: %s" $errormsg ]
+} else {
+ set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+</programlisting>
+ If the second <literal>UPDATE</literal> statement results in an
+ exception being raised, this function will report the error, but
+ the result of the first <literal>UPDATE</literal> will
+ nevertheless be committed. In other words, the funds will be
+ withdrawn from Joe's account, but will not be transferred to
+ Mary's account.
+ </para>
+
+ <para>
+ To avoid such issues, you can wrap your
+ <literal>spi_exec</literal> calls in an explicit
+ subtransaction. The PL/Tcl provides a
+ commmand <literal>subtransaction</literal> to manage explicit
+ subtransactions.
+ Using explicit subtransactions
+ we can rewrite our function as:
+<programlisting>
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+ subtransaction {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+ }
+ } errormsg] {
+ set result [format "error transferring funds: %s" $errormsg]
+ } else {
+ set result "funds transferred correctly"
+ }
+ set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+ spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+</programlisting>
+ Note that the use of <literal>catch</literal> is still
+ required. Otherwise the exception would propagate to the top of
+ the stack and would cause the whole function to abort with
+ a <productname>PostgreSQL</productname> error, so that the
+ <literal>operations</literal> table would not have any row
+ inserted into it. The <literal>subtransaction</literal> command does not
+ trap errors, it only assures that all database operations executed
+ inside its scope will be atomically committed or rolled back. A
+ rollback of the subtransaction block occurs on any kind of
+ exception exit, not only ones caused by errors originating from
+ database access. A regular Tcl exception raised inside an
+ explicit subtransaction block would also cause the subtransaction
+ to be rolled back.
+ </para>
+ </sect2>
+ </sect1>
<sect1 id="pltcl-unknown">
<title>Modules and the <function>unknown</> Command</title>
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 25082ec..614385d 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_unicode pltcl_subxact
# Tcl on win32 ships with import libraries only for Microsoft Visual C++,
# which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/expected/pltcl_subxact.out b/src/pl/tcl/expected/pltcl_subxact.out
new file mode 100644
index 0000000..17b9d90
--- /dev/null
+++ b/src/pl/tcl/expected/pltcl_subxact.out
@@ -0,0 +1,145 @@
+--
+-- Test explicit subtransactions
+--
+-- Test table to see if transactions get properly rolled back
+CREATE TABLE subtransaction_tbl (
+ i integer
+);
+CREATE FUNCTION subtransaction_ctx_test(what_error text = NULL) RETURNS text
+AS $$
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"} {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')"
+ } elseif { $1 == "Tcl"} {
+ elog ERROR "Tcl error"
+ }
+}
+$$ LANGUAGE pltcl;
+SELECT subtransaction_ctx_test();
+ subtransaction_ctx_test
+-------------------------
+
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 1
+ 2
+(2 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('SPI');
+ERROR: invalid input syntax for integer: "oops"
+CONTEXT: invalid input syntax for integer: "oops"
+ while executing
+"spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')""
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"..."
+ (procedure "__PLTcl_proc_16494" line 3)
+ invoked from within
+"__PLTcl_proc_16494 SPI"
+in PL/Tcl function "subtransaction_ctx_test"
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('Tcl');
+ERROR: Tcl error
+CONTEXT: Tcl error
+ while executing
+"elog ERROR "Tcl error""
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"..."
+ (procedure "__PLTcl_proc_16494" line 3)
+ invoked from within
+"__PLTcl_proc_16494 Tcl"
+in PL/Tcl function "subtransaction_ctx_test"
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+-- Nested subtransactions
+CREATE FUNCTION subtransaction_nested_test(swallow boolean = 'f') RETURNS text
+AS $$
+elog NOTICE "subtransaction_tested_test got arg '$1'"
+spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+ if {$1 != "t"} {
+ error $errormsg $::errorInfo $::errorCode
+ }
+ elog NOTICE "Swallowed $errormsg"
+ }
+}
+return "ok"
+$$ LANGUAGE pltcl;
+SELECT subtransaction_nested_test();
+NOTICE: subtransaction_tested_test got arg 'f'
+ERROR: syntax error at or near "error"
+CONTEXT: syntax error at or near "error"
+ while executing
+"spi_exec "error""
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }"
+ invoked from within
+"if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+..."
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subt..."
+ (procedure "__PLTcl_proc_16498" line 5)
+ invoked from within
+"__PLTcl_proc_16498 f"
+in PL/Tcl function "subtransaction_nested_test"
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_nested_test('t');
+NOTICE: subtransaction_tested_test got arg 't'
+NOTICE: Swallowed syntax error at or near "error"
+ subtransaction_nested_test
+----------------------------
+ ok
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 1
+ 2
+(2 rows)
+
+TRUNCATE subtransaction_tbl;
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index d813dcb..f03ecf1 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -307,6 +307,8 @@ static HeapTuple pltcl_build_tuple_result(Tcl_Interp *interp,
pltcl_call_state *call_state);
static void pltcl_init_tuple_store(pltcl_call_state *call_state);
+static int pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[]);
/*
* Hack to override Tcl's builtin Notifier subsystem. This prevents the
@@ -483,7 +485,8 @@ pltcl_init_interp(pltcl_interp_desc *interp_desc, bool pltrusted)
pltcl_SPI_execute_plan, NULL, NULL);
Tcl_CreateObjCommand(interp, "spi_lastoid",
pltcl_SPI_lastoid, NULL, NULL);
-
+ Tcl_CreateObjCommand(interp, "subtransaction",
+ pltcl_subtransaction, NULL,NULL);
/************************************************************
* Try to load the unknown procedure from pltcl_modules
************************************************************/
@@ -3070,3 +3073,43 @@ pltcl_init_tuple_store(pltcl_call_state *call_state)
CurrentResourceOwner = oldowner;
MemoryContextSwitchTo(oldcxt);
}
+
+/*
+ * pltcl_subtransaction - implements tcl level subtransaction block
+ * Called with exactly one argument - piece of Tcl Code, and executes
+ * this code inside subtransaction.
+ * Rolls subtransaction back if Tcl_EvalEx returns TCL_ERROR
+ */
+static int
+pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[])
+{
+ /* Initialize local memory context */
+ MemoryContext oldcontext = CurrentMemoryContext;
+ ResourceOwner oldowner = CurrentResourceOwner;
+ int retcode;
+ if (objc != 2)
+ {
+ Tcl_WrongNumArgs(interp,1,objv,"command");
+ return TCL_ERROR;
+ }
+
+ BeginInternalSubTransaction(NULL);
+ MemoryContextSwitchTo(TopTransactionContext);
+
+ retcode = Tcl_EvalObjEx(interp, objv[1],0);
+
+ if (retcode == TCL_ERROR)
+ {
+ /* Roollback the subtransaction */
+ RollbackAndReleaseCurrentSubTransaction();
+ }
+ else
+ {
+ /* Commit the subtransaction */
+ ReleaseCurrentSubTransaction();
+ }
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+ return retcode;
+}
diff --git a/src/pl/tcl/sql/pltcl_subxact.sql b/src/pl/tcl/sql/pltcl_subxact.sql
new file mode 100644
index 0000000..c5859e1
--- /dev/null
+++ b/src/pl/tcl/sql/pltcl_subxact.sql
@@ -0,0 +1,63 @@
+--
+-- Test explicit subtransactions
+--
+
+-- Test table to see if transactions get properly rolled back
+
+CREATE TABLE subtransaction_tbl (
+ i integer
+);
+
+CREATE FUNCTION subtransaction_ctx_test(what_error text = NULL) RETURNS text
+AS $$
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"} {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')"
+ } elseif { $1 == "Tcl"} {
+ elog ERROR "Tcl error"
+ }
+}
+$$ LANGUAGE pltcl;
+
+SELECT subtransaction_ctx_test();
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('SPI');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('Tcl');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+-- Nested subtransactions
+
+CREATE FUNCTION subtransaction_nested_test(swallow boolean = 'f') RETURNS text
+AS $$
+elog NOTICE "subtransaction_tested_test got arg '$1'"
+spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+ if {$1 != "t"} {
+ error $errormsg $::errorInfo $::errorCode
+ }
+ elog NOTICE "Swallowed $errormsg"
+ }
+}
+return "ok"
+$$ LANGUAGE pltcl;
+
+SELECT subtransaction_nested_test();
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+SELECT subtransaction_nested_test('t');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
Victor Wagner <vitus@wagner.pp.ru> writes:
I'm attaching the patch which implements subtransaction command for
PL/Tcl which does almost same as PL/Python plpy.subtransction context
manager object does: executes a block of Tcl code in the context of
subtransaction, rolls subtransaction back if error occures and commits
it otherwise.
Interesting. Please add this to the 2017-03 commitfest list, to make
sure we don't lose track of it.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2017-01-08 18:57 GMT+01:00 Victor Wagner <vitus@wagner.pp.ru>:
Collegues!
Recently I've found out that PL/Python have very nice feature - explicit
subtransaction object, which allows to execute block of code in the
context of subtransaction.I've quite surprised that other PL languages, shipped with PostgreSQL do
not have such useful construction.If it might require considerable trickery to add such functionality into
PL/Perl, Tcl allows to add new control stuctures very easily.I'm attaching the patch which implements subtransaction command for
PL/Tcl which does almost same as PL/Python plpy.subtransction context
manager object does: executes a block of Tcl code in the context of
subtransaction, rolls subtransaction back if error occures and commits
it otherwise.It looks like
subtransaction {
...some Tcl code...
}Typically one would use it inside Tcl catch statement:
if [catch {
subtransaction {
spi_exec "insert into..."
...
}
} errormsg] {
# Handle an error
}See documentation and tests included in the patch for more complete
examples.Just like corresponding Python construction, this command doesn't
replace language builtin exception handling, just adds subtransaction
support to it.Patch includes sufficiently less tests than python subtransaction tests,
because Tcl implementation is way simpler than python one, and doesn't
have syntactic variatons which depend on language version.Also entering and exiting subtransactions are in the same piece of code
rather than in separate __enter__ and __exit__ methods as in Python, so
there is no possibility to call exit without enter or vice versa.
I did a review of this patch
1. This functionality has sense and nobody was against this feature.
2. This patch does what is proposed - it introduce new TCL function that
wraps PostgreSQL subtransaction
3. This patch is really simple due massive using subtransactions already -
every SPI called from TCL is wrapped to subtransaction.
4. A documentation is good - although I am not sure if it is well
structured - is <sect2> level necessary? Probably there will not be any
other similar command.
5. There are a basic regress tests, and all tests passed, but I miss a
path, where subtransaction is commited - now rollback is every time
6. The code has some issues with white chars
7. I don't understand why TopMemoryContext is used there? Maybe some
already used context should be there.
+<->BeginInternalSubTransaction(NULL);
+<->MemoryContextSwitchTo(TopTransactionContext);
+<->Regards
Pavel
Show quoted text
--
Victor Wagner <vitus@wagner.pp.ru>
On Wed, 8 Mar 2017 16:49:33 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:
I did a review of this patch
First, thank you for you effort. I already begin to think that nobody
is really interesting in PL/Tcl stuff.
1. This functionality has sense and nobody was against this feature.
2. This patch does what is proposed - it introduce new TCL function
that wraps PostgreSQL subtransaction3. This patch is really simple due massive using subtransactions
already - every SPI called from TCL is wrapped to subtransaction.4. A documentation is good - although I am not sure if it is well
structured - is <sect2> level necessary? Probably there will not be
any other similar command.
You are right. At least sect2 can be added later whenever this second
command will appear.
5. There are a basic regress tests, and all tests passed, but I miss a
path, where subtransaction is commited - now rollback is every time
Really, I haven't found such tests in PL/Python suite, so I haven't
translated it to Tcl.
It might be good idea to add such test.
6. The code has some issues with white chars
7. I don't understand why TopMemoryContext is used there? Maybe some
already used context should be there.+<->BeginInternalSubTransaction(NULL); +<->MemoryContextSwitchTo(TopTransactionContext); +<->
I've copied this code from PL/Python subtransaction object and
it has following comment:
/* Be sure that cells of explicit_subtransactions list are long-lived */
But in Tcl case wi don't have to maintain our own stack in the form of
list. We use C-language stack and keep our data in the local variables.
So, probably changing of memory contexts is not necessary at all.
But it require a bit more investigation and testing.
With best regards, Victor
--
Victor Wagner <vitus@wagner.pp.ru>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, 8 Mar 2017 16:49:33 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:
I did a review of this patch
I'm attaching new version of patch with the issues pointed by you fixed.
4. A documentation is good - although I am not sure if it is well
structured - is <sect2> level necessary? Probably there will not be
any other similar command.
Removed <sect2>. Although it is questionable - now there is no
subsection with name of the command in the title. But surrounding
section describes only one command,.
5. There are a basic regress tests, and all tests passed, but I miss a
path, where subtransaction is commited - now rollback is every time
Added test with successful subtransaction commit. Just to be sure it is
really commited.
6. The code has some issues with white chars
Fixed where I've found it. Now, hopefully my code uses same identation
rules as other pltcl.c
7. I don't understand why TopMemoryContext is used there? Maybe some
already used context should be there.+<->BeginInternalSubTransaction(NULL); +<->MemoryContextSwitchTo(TopTransactionContext); +<->
It turns out that was really an error in the first version of patch.
This context manipulation was copied from PL/Python context manager by
mistake.
PL/Python changes to TopTransactionContext in order to manage
stack of subtransaction data (which we don't need in Tcl, because we
can use C language stack and store neccessary data in function
automatic variables. Really python doesn't need this stack to, because
Python context manager is a python language object and can keep state
inside it).
Although we still need to keep MemoryContext and ResourceOwner and
restore them upon transaction finish.
With best regards, Victor
--
Victor Wagner <vitus@wagner.pp.ru>
Attachments:
tcl_subtransaction_02.patchtext/x-patchDownload
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index ad216dd..87bc4ad 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,79 @@ if {[catch { spi_exec $sql_command }]} {
is a global variable.)
</para>
</sect1>
+ <sect1 id="pltcl-subtransaction">
+ <title>Explicit Subtransactions</title>
+ <para>
+ Recovering from errors caused by database access as described in
+ <xref linkend="pltcl-error-handling"> can lead to an undesirable
+ situation where some operations succeed before one of them fails,
+ and after recovering from that error the data is left in an
+ inconsistent state. PL/Tcl offers a solution to this problem in
+ the form of explicit subtransactions.
+ </para>
+ <para>
+ Consider a function that implements a transfer between two
+ accounts:
+<programlisting>
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+ set result [format "error transferring funds: %s" $errormsg ]
+} else {
+ set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+</programlisting>
+ If the second <literal>UPDATE</literal> statement results in an
+ exception being raised, this function will report the error, but
+ the result of the first <literal>UPDATE</literal> will
+ nevertheless be committed. In other words, the funds will be
+ withdrawn from Joe's account, but will not be transferred to
+ Mary's account.
+ </para>
+ <para>
+ To avoid such issues, you can wrap your
+ <literal>spi_exec</literal> calls in an explicit
+ subtransaction. The PL/Tcl provides a
+ commmand <literal>subtransaction</literal> to manage explicit
+ subtransactions.
+ Using explicit subtransactions
+ we can rewrite our function as:
+<programlisting>
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+ subtransaction {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+ }
+ } errormsg] {
+ set result [format "error transferring funds: %s" $errormsg]
+ } else {
+ set result "funds transferred correctly"
+ }
+ set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+ spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+</programlisting>
+ Note that the use of <literal>catch</literal> is still
+ required. Otherwise the exception would propagate to the top of
+ the stack and would cause the whole function to abort with
+ a <productname>PostgreSQL</productname> error, so that the
+ <literal>operations</literal> table would not have any row
+ inserted into it. The <literal>subtransaction</literal> command does not
+ trap errors, it only assures that all database operations executed
+ inside its scope will be atomically committed or rolled back. A
+ rollback of the subtransaction block occurs on any kind of
+ exception exit, not only ones caused by errors originating from
+ database access. A regular Tcl exception raised inside an
+ explicit subtransaction block would also cause the subtransaction
+ to be rolled back.
+ </para>
+ </sect1>
<sect1 id="pltcl-config">
<title>PL/Tcl Configuration</title>
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 1096c4f..b6b6b19 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode pltcl_subxact
# Tcl on win32 ships with import libraries only for Microsoft Visual C++,
# which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 2cf7e66..8d498ae 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -323,6 +323,8 @@ static HeapTuple pltcl_build_tuple_result(Tcl_Interp *interp,
pltcl_call_state *call_state);
static void pltcl_init_tuple_store(pltcl_call_state *call_state);
+static int pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[]);
/*
* Hack to override Tcl's builtin Notifier subsystem. This prevents the
@@ -516,7 +518,8 @@ pltcl_init_interp(pltcl_interp_desc *interp_desc, Oid prolang, bool pltrusted)
pltcl_SPI_execute_plan, NULL, NULL);
Tcl_CreateObjCommand(interp, "spi_lastoid",
pltcl_SPI_lastoid, NULL, NULL);
-
+ Tcl_CreateObjCommand(interp, "subtransaction",
+ pltcl_subtransaction, NULL,NULL);
/************************************************************
* Call the appropriate start_proc, if there is one.
*
@@ -3114,3 +3117,44 @@ pltcl_init_tuple_store(pltcl_call_state *call_state)
CurrentResourceOwner = oldowner;
MemoryContextSwitchTo(oldcxt);
}
+
+/*
+ * pltcl_subtransaction - implements tcl level subtransaction block
+ * Called with exactly one argument - piece of Tcl Code, and executes
+ * this code inside subtransaction.
+ * Rolls subtransaction back if Tcl_EvalEx returns TCL_ERROR
+ */
+static int
+pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[])
+{
+ /* Save resource owner and memory context in the local vars */
+ ResourceOwner oldowner = CurrentResourceOwner;
+ MemoryContext oldcontext = CurrentMemoryContext;
+ int retcode;
+ if (objc != 2)
+ {
+ Tcl_WrongNumArgs(interp,1,objv,"command");
+ return TCL_ERROR;
+ }
+
+ BeginInternalSubTransaction(NULL);
+
+ retcode = Tcl_EvalObjEx(interp, objv[1],0);
+
+ if (retcode == TCL_ERROR)
+ {
+ /* Roollback the subtransaction */
+ RollbackAndReleaseCurrentSubTransaction();
+ }
+ else
+ {
+ /* Commit the subtransaction */
+ ReleaseCurrentSubTransaction();
+ }
+ /* Restore resource owner and memory context
+ In case they were changed inside subtransaction */
+ CurrentResourceOwner = oldowner;
+ MemoryContextSwitchTo(oldcontext);
+ return retcode;
+}
2017-03-09 7:48 GMT+01:00 Victor Wagner <vitus@wagner.pp.ru>:
On Wed, 8 Mar 2017 16:49:33 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:I did a review of this patch
I'm attaching new version of patch with the issues pointed by you fixed.
4. A documentation is good - although I am not sure if it is well
structured - is <sect2> level necessary? Probably there will not be
any other similar command.Removed <sect2>. Although it is questionable - now there is no
subsection with name of the command in the title. But surrounding
section describes only one command,.5. There are a basic regress tests, and all tests passed, but I miss a
path, where subtransaction is commited - now rollback is every timeAdded test with successful subtransaction commit. Just to be sure it is
really commited.6. The code has some issues with white chars
Fixed where I've found it. Now, hopefully my code uses same identation
rules as other pltcl.c7. I don't understand why TopMemoryContext is used there? Maybe some
already used context should be there.+<->BeginInternalSubTransaction(NULL); +<->MemoryContextSwitchTo(TopTransactionContext); +<->It turns out that was really an error in the first version of patch.
This context manipulation was copied from PL/Python context manager by
mistake.PL/Python changes to TopTransactionContext in order to manage
stack of subtransaction data (which we don't need in Tcl, because we
can use C language stack and store neccessary data in function
automatic variables. Really python doesn't need this stack to, because
Python context manager is a python language object and can keep state
inside it).Although we still need to keep MemoryContext and ResourceOwner and
restore them upon transaction finish.
is this patch complete? I don't see new regress tests
Regards
Pavel
Show quoted text
With best regards, Victor
--
Victor Wagner <vitus@wagner.pp.ru>
On Thu, 9 Mar 2017 09:25:14 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:
is this patch complete? I don't see new regress tests
Oh, really! I've forgot that git diff doesn't include files which are
not added into git.
So, no old regress tests as well.
Sorry for posting incomplete patch.
Attached fixed version of patch with regress tests and couple more
whitespace issues fixed.
With best regards, Victor
--
Victor Wagner <vitus@wagner.pp.ru>
Attachments:
tcl_subtransaction_03.patchtext/x-patchDownload
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index ad216dd..87bc4ad 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,79 @@ if {[catch { spi_exec $sql_command }]} {
is a global variable.)
</para>
</sect1>
+ <sect1 id="pltcl-subtransaction">
+ <title>Explicit Subtransactions</title>
+ <para>
+ Recovering from errors caused by database access as described in
+ <xref linkend="pltcl-error-handling"> can lead to an undesirable
+ situation where some operations succeed before one of them fails,
+ and after recovering from that error the data is left in an
+ inconsistent state. PL/Tcl offers a solution to this problem in
+ the form of explicit subtransactions.
+ </para>
+ <para>
+ Consider a function that implements a transfer between two
+ accounts:
+<programlisting>
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+ set result [format "error transferring funds: %s" $errormsg ]
+} else {
+ set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+</programlisting>
+ If the second <literal>UPDATE</literal> statement results in an
+ exception being raised, this function will report the error, but
+ the result of the first <literal>UPDATE</literal> will
+ nevertheless be committed. In other words, the funds will be
+ withdrawn from Joe's account, but will not be transferred to
+ Mary's account.
+ </para>
+ <para>
+ To avoid such issues, you can wrap your
+ <literal>spi_exec</literal> calls in an explicit
+ subtransaction. The PL/Tcl provides a
+ commmand <literal>subtransaction</literal> to manage explicit
+ subtransactions.
+ Using explicit subtransactions
+ we can rewrite our function as:
+<programlisting>
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+ subtransaction {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+ }
+ } errormsg] {
+ set result [format "error transferring funds: %s" $errormsg]
+ } else {
+ set result "funds transferred correctly"
+ }
+ set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+ spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+</programlisting>
+ Note that the use of <literal>catch</literal> is still
+ required. Otherwise the exception would propagate to the top of
+ the stack and would cause the whole function to abort with
+ a <productname>PostgreSQL</productname> error, so that the
+ <literal>operations</literal> table would not have any row
+ inserted into it. The <literal>subtransaction</literal> command does not
+ trap errors, it only assures that all database operations executed
+ inside its scope will be atomically committed or rolled back. A
+ rollback of the subtransaction block occurs on any kind of
+ exception exit, not only ones caused by errors originating from
+ database access. A regular Tcl exception raised inside an
+ explicit subtransaction block would also cause the subtransaction
+ to be rolled back.
+ </para>
+ </sect1>
<sect1 id="pltcl-config">
<title>PL/Tcl Configuration</title>
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 1096c4f..b6b6b19 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode pltcl_subxact
# Tcl on win32 ships with import libraries only for Microsoft Visual C++,
# which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/expected/pltcl_subxact.out b/src/pl/tcl/expected/pltcl_subxact.out
new file mode 100644
index 0000000..5d984f1
--- /dev/null
+++ b/src/pl/tcl/expected/pltcl_subxact.out
@@ -0,0 +1,171 @@
+--
+-- Test explicit subtransactions
+--
+CREATE TABLE subtransaction_tbl (
+ i integer
+);
+-- test subtransaction successfully commited
+CREATE FUNCTION subtransaction_ctx_success() RETURNS text
+AS $$
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(1)"
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(2)"
+ }
+$$ LANGUAGE pltcl;
+BEGIN;
+INSERT INTO subtransaction_tbl VALUES(0);
+SELECT subtransaction_ctx_success();
+ subtransaction_ctx_success
+----------------------------
+ 1
+(1 row)
+
+COMMIT;
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 0
+ 1
+ 2
+(3 rows)
+
+TRUNCATE subtransaction_tbl;
+-- Test table to see if transactions get properly rolled back
+CREATE FUNCTION subtransaction_ctx_test(what_error text = NULL) RETURNS text
+AS $$
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"} {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')"
+ } elseif { $1 == "Tcl"} {
+ elog ERROR "Tcl error"
+ }
+}
+$$ LANGUAGE pltcl;
+SELECT subtransaction_ctx_test();
+ subtransaction_ctx_test
+-------------------------
+
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 1
+ 2
+(2 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('SPI');
+ERROR: invalid input syntax for integer: "oops"
+CONTEXT: invalid input syntax for integer: "oops"
+ while executing
+"spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')""
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"..."
+ (procedure "__PLTcl_proc_16503" line 3)
+ invoked from within
+"__PLTcl_proc_16503 SPI"
+in PL/Tcl function "subtransaction_ctx_test"
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('Tcl');
+ERROR: Tcl error
+CONTEXT: Tcl error
+ while executing
+"elog ERROR "Tcl error""
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"..."
+ (procedure "__PLTcl_proc_16503" line 3)
+ invoked from within
+"__PLTcl_proc_16503 Tcl"
+in PL/Tcl function "subtransaction_ctx_test"
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+-- Nested subtransactions
+CREATE FUNCTION subtransaction_nested_test(swallow boolean = 'f') RETURNS text
+AS $$
+elog NOTICE "subtransaction_tested_test got arg '$1'"
+spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+ if {$1 != "t"} {
+ error $errormsg $::errorInfo $::errorCode
+ }
+ elog NOTICE "Swallowed $errormsg"
+ }
+}
+return "ok"
+$$ LANGUAGE pltcl;
+SELECT subtransaction_nested_test();
+NOTICE: subtransaction_tested_test got arg 'f'
+ERROR: syntax error at or near "error"
+CONTEXT: syntax error at or near "error"
+ while executing
+"spi_exec "error""
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }"
+ invoked from within
+"if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+..."
+ invoked from within
+"subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subt..."
+ (procedure "__PLTcl_proc_16507" line 5)
+ invoked from within
+"__PLTcl_proc_16507 f"
+in PL/Tcl function "subtransaction_nested_test"
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_nested_test('t');
+NOTICE: subtransaction_tested_test got arg 't'
+NOTICE: Swallowed syntax error at or near "error"
+ subtransaction_nested_test
+----------------------------
+ ok
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 1
+ 2
+(2 rows)
+
+TRUNCATE subtransaction_tbl;
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 2cf7e66..266d7b9 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -323,6 +323,8 @@ static HeapTuple pltcl_build_tuple_result(Tcl_Interp *interp,
pltcl_call_state *call_state);
static void pltcl_init_tuple_store(pltcl_call_state *call_state);
+static int pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[]);
/*
* Hack to override Tcl's builtin Notifier subsystem. This prevents the
@@ -516,7 +518,8 @@ pltcl_init_interp(pltcl_interp_desc *interp_desc, Oid prolang, bool pltrusted)
pltcl_SPI_execute_plan, NULL, NULL);
Tcl_CreateObjCommand(interp, "spi_lastoid",
pltcl_SPI_lastoid, NULL, NULL);
-
+ Tcl_CreateObjCommand(interp, "subtransaction",
+ pltcl_subtransaction, NULL,NULL);
/************************************************************
* Call the appropriate start_proc, if there is one.
*
@@ -3114,3 +3117,44 @@ pltcl_init_tuple_store(pltcl_call_state *call_state)
CurrentResourceOwner = oldowner;
MemoryContextSwitchTo(oldcxt);
}
+
+/*
+ * pltcl_subtransaction - implements tcl level subtransaction block
+ * Called with exactly one argument - piece of Tcl Code, and executes
+ * this code inside subtransaction.
+ * Rolls subtransaction back if Tcl_EvalEx returns TCL_ERROR
+ */
+static int
+pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[])
+{
+ /* Save resource owner and memory context in the local vars */
+ ResourceOwner oldowner = CurrentResourceOwner;
+ MemoryContext oldcontext = CurrentMemoryContext;
+ int retcode;
+ if (objc != 2)
+ {
+ Tcl_WrongNumArgs(interp,1,objv,"command");
+ return TCL_ERROR;
+ }
+
+ BeginInternalSubTransaction(NULL);
+
+ retcode = Tcl_EvalObjEx(interp, objv[1],0);
+
+ if (retcode == TCL_ERROR)
+ {
+ /* Roollback the subtransaction */
+ RollbackAndReleaseCurrentSubTransaction();
+ }
+ else
+ {
+ /* Commit the subtransaction */
+ ReleaseCurrentSubTransaction();
+ }
+ /* Restore resource owner and memory context
+ In case they were changed inside subtransaction */
+ CurrentResourceOwner = oldowner;
+ MemoryContextSwitchTo(oldcontext);
+ return retcode;
+}
diff --git a/src/pl/tcl/sql/pltcl_subxact.sql b/src/pl/tcl/sql/pltcl_subxact.sql
new file mode 100644
index 0000000..b243083
--- /dev/null
+++ b/src/pl/tcl/sql/pltcl_subxact.sql
@@ -0,0 +1,81 @@
+--
+-- Test explicit subtransactions
+--
+
+
+CREATE TABLE subtransaction_tbl (
+ i integer
+);
+
+-- test subtransaction successfully commited
+
+CREATE FUNCTION subtransaction_ctx_success() RETURNS text
+AS $$
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(1)"
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(2)"
+ }
+$$ LANGUAGE pltcl;
+
+BEGIN;
+INSERT INTO subtransaction_tbl VALUES(0);
+SELECT subtransaction_ctx_success();
+COMMIT;
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+-- Test table to see if transactions get properly rolled back
+
+CREATE FUNCTION subtransaction_ctx_test(what_error text = NULL) RETURNS text
+AS $$
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"} {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')"
+ } elseif { $1 == "Tcl"} {
+ elog ERROR "Tcl error"
+ }
+}
+$$ LANGUAGE pltcl;
+
+SELECT subtransaction_ctx_test();
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('SPI');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+SELECT subtransaction_ctx_test('Tcl');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+-- Nested subtransactions
+
+CREATE FUNCTION subtransaction_nested_test(swallow boolean = 'f') RETURNS text
+AS $$
+elog NOTICE "subtransaction_tested_test got arg '$1'"
+spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+ if {$1 != "t"} {
+ error $errormsg $::errorInfo $::errorCode
+ }
+ elog NOTICE "Swallowed $errormsg"
+ }
+}
+return "ok"
+$$ LANGUAGE pltcl;
+
+SELECT subtransaction_nested_test();
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+SELECT subtransaction_nested_test('t');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
Hi
2017-03-09 10:25 GMT+01:00 Victor Wagner <vitus@wagner.pp.ru>:
On Thu, 9 Mar 2017 09:25:14 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:is this patch complete? I don't see new regress tests
Oh, really! I've forgot that git diff doesn't include files which are
not added into git.So, no old regress tests as well.
Sorry for posting incomplete patch.
Attached fixed version of patch with regress tests and couple more
whitespace issues fixed.
the regress tests is unstable
the proc name has mutable pid
! (procedure "__PLTcl_proc_16503" line 3)
invoked from within
! "__PLTcl_proc_16503 SPI"
Regards
Pavel
Show quoted text
With best regards, Victor
--
Victor Wagner <vitus@wagner.pp.ru>
On Thu, 9 Mar 2017 11:12:09 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:
the regress tests is unstable
the proc name has mutable pid
! (procedure "__PLTcl_proc_16503" line 3)
invoked from within
! "__PLTcl_proc_16503 SPI"Regards
Really, I don't know what can be done with it, short of rewriting all
tests as tap-tests.
Definitely this patch is not the right place for reversing desing
decision of PL/Tcl authors to add a numeric suffix to the proc names.
(it is not PID. It is OID of the function).
Of course, I can catch all the errors inside Tcl code and return
just message, but it would sufficiently narrow test functionality.
Now test demonstrate how errors uncaught on the Tcl level interact with
postgresql error system.
With best regards, Victor
--
Victor Wagner <vitus@wagner.pp.ru>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2017-03-09 11:45 GMT+01:00 Victor Wagner <vitus@wagner.pp.ru>:
On Thu, 9 Mar 2017 11:12:09 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:the regress tests is unstable
the proc name has mutable pid
! (procedure "__PLTcl_proc_16503" line 3)
invoked from within
! "__PLTcl_proc_16503 SPI"Regards
Really, I don't know what can be done with it, short of rewriting all
tests as tap-tests.Definitely this patch is not the right place for reversing desing
decision of PL/Tcl authors to add a numeric suffix to the proc names.
(it is not PID. It is OID of the function).Of course, I can catch all the errors inside Tcl code and return
just message, but it would sufficiently narrow test functionality.Now test demonstrate how errors uncaught on the Tcl level interact with
postgresql error system.
you can catch the exception outside and write own message
Pavel
Show quoted text
With best regards, Victor
--
Victor Wagner <vitus@wagner.pp.ru>--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, 9 Mar 2017 12:04:31 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:
Now test demonstrate how errors uncaught on the Tcl level interact
with postgresql error system.you can catch the exception outside and write own message
OK, here is patch with tests which don't depend on function OIDs
They ignore stack trace ($::errorinfo global variable) completely,
and analyze just error message.
--
Victor Wagner <vitus@wagner.pp.ru>
Attachments:
tcl_subtransaction_04.patchtext/x-patchDownload
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index ad216dd..87bc4ad 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -901,6 +901,79 @@ if {[catch { spi_exec $sql_command }]} {
is a global variable.)
</para>
</sect1>
+ <sect1 id="pltcl-subtransaction">
+ <title>Explicit Subtransactions</title>
+ <para>
+ Recovering from errors caused by database access as described in
+ <xref linkend="pltcl-error-handling"> can lead to an undesirable
+ situation where some operations succeed before one of them fails,
+ and after recovering from that error the data is left in an
+ inconsistent state. PL/Tcl offers a solution to this problem in
+ the form of explicit subtransactions.
+ </para>
+ <para>
+ Consider a function that implements a transfer between two
+ accounts:
+<programlisting>
+CREATE FUNCTION transfer_funds() RETURNS void AS $$
+if [catch {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+} errormsg] {
+ set result [format "error transferring funds: %s" $errormsg ]
+} else {
+ set result "funds transferred correctly"
+}
+set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+spi_execp -count 1 $plan, [list $result)
+$$ LANGUAGE pltclu;
+</programlisting>
+ If the second <literal>UPDATE</literal> statement results in an
+ exception being raised, this function will report the error, but
+ the result of the first <literal>UPDATE</literal> will
+ nevertheless be committed. In other words, the funds will be
+ withdrawn from Joe's account, but will not be transferred to
+ Mary's account.
+ </para>
+ <para>
+ To avoid such issues, you can wrap your
+ <literal>spi_exec</literal> calls in an explicit
+ subtransaction. The PL/Tcl provides a
+ commmand <literal>subtransaction</literal> to manage explicit
+ subtransactions.
+ Using explicit subtransactions
+ we can rewrite our function as:
+<programlisting>
+CREATE FUNCTION transfer_funds2() RETURNS void AS $$
+if [catch {
+ subtransaction {
+ spi_exec "UPDATE accounts SET balance = balance - 100 WHERE account_name = 'joe'"
+ spi_exec "UPDATE accounts SET balance = balance + 100 WHERE account_name = 'mary'"
+ }
+ } errormsg] {
+ set result [format "error transferring funds: %s" $errormsg]
+ } else {
+ set result "funds transferred correctly"
+ }
+ set plan [spi_prepare "INSERT INTO operations (result) VALUES ($1)"]
+ spi_execp $plan, [list $result]
+$$ LANGUAGE pltclu;
+</programlisting>
+ Note that the use of <literal>catch</literal> is still
+ required. Otherwise the exception would propagate to the top of
+ the stack and would cause the whole function to abort with
+ a <productname>PostgreSQL</productname> error, so that the
+ <literal>operations</literal> table would not have any row
+ inserted into it. The <literal>subtransaction</literal> command does not
+ trap errors, it only assures that all database operations executed
+ inside its scope will be atomically committed or rolled back. A
+ rollback of the subtransaction block occurs on any kind of
+ exception exit, not only ones caused by errors originating from
+ database access. A regular Tcl exception raised inside an
+ explicit subtransaction block would also cause the subtransaction
+ to be rolled back.
+ </para>
+ </sect1>
<sect1 id="pltcl-config">
<title>PL/Tcl Configuration</title>
diff --git a/src/pl/tcl/Makefile b/src/pl/tcl/Makefile
index 1096c4f..b6b6b19 100644
--- a/src/pl/tcl/Makefile
+++ b/src/pl/tcl/Makefile
@@ -28,7 +28,7 @@ DATA = pltcl.control pltcl--1.0.sql pltcl--unpackaged--1.0.sql \
pltclu.control pltclu--1.0.sql pltclu--unpackaged--1.0.sql
REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-extension=pltcl
-REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode
+REGRESS = pltcl_setup pltcl_queries pltcl_start_proc pltcl_unicode pltcl_subxact
# Tcl on win32 ships with import libraries only for Microsoft Visual C++,
# which are not compatible with mingw gcc. Therefore we need to build a
diff --git a/src/pl/tcl/expected/pltcl_subxact.out b/src/pl/tcl/expected/pltcl_subxact.out
new file mode 100644
index 0000000..3ea3ef3
--- /dev/null
+++ b/src/pl/tcl/expected/pltcl_subxact.out
@@ -0,0 +1,146 @@
+--
+-- Test explicit subtransactions
+--
+CREATE TABLE subtransaction_tbl (
+ i integer
+);
+-- test subtransaction successfully commited
+CREATE FUNCTION subtransaction_ctx_success() RETURNS text
+AS $$
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(1)"
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(2)"
+ }
+$$ LANGUAGE pltcl;
+BEGIN;
+INSERT INTO subtransaction_tbl VALUES(0);
+SELECT subtransaction_ctx_success();
+ subtransaction_ctx_success
+----------------------------
+ 1
+(1 row)
+
+COMMIT;
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 0
+ 1
+ 2
+(3 rows)
+
+TRUNCATE subtransaction_tbl;
+-- Test table to see if transactions get properly rolled back
+CREATE FUNCTION subtransaction_ctx_test(what_error text = NULL) RETURNS text
+AS $$
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"} {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')"
+ } elseif { $1 == "Tcl"} {
+ elog ERROR "Tcl error"
+ }
+}
+$$ LANGUAGE pltcl;
+--
+-- We use this wrapper to catch errors and return errormsg only,
+-- because values of $::errorinfo variable contain procedure name which
+-- includes OID, which may differ during make installcheck
+--
+CREATE FUNCTION pltcl_wrapper(statement text) RETURNS text
+AS $$
+ if [catch {spi_exec $1} msg] {
+ return "ERROR: $msg"
+ } else {
+ return "SUCCESS: $msg"
+ }
+$$ LANGUAGE pltcl;
+SELECT pltcl_wrapper('SELECT subtransaction_ctx_test()');
+ pltcl_wrapper
+---------------
+ SUCCESS: 1
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 1
+ 2
+(2 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT pltcl_wrapper('SELECT subtransaction_ctx_test(''SPI'')');
+ pltcl_wrapper
+-------------------------------------------------
+ ERROR: invalid input syntax for integer: "oops"
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT pltcl_wrapper('SELECT subtransaction_ctx_test(''Tcl'')');
+ pltcl_wrapper
+------------------
+ ERROR: Tcl error
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+-- Nested subtransactions
+CREATE FUNCTION subtransaction_nested_test(swallow boolean = 'f') RETURNS text
+AS $$
+elog NOTICE "subtransaction_tested_test got arg '$1'"
+spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+ if {$1 != "t"} {
+ error $errormsg $::errorInfo $::errorCode
+ }
+ elog NOTICE "Swallowed $errormsg"
+ }
+}
+return "ok"
+$$ LANGUAGE pltcl;
+SELECT pltcl_wrapper('SELECT subtransaction_nested_test()');
+NOTICE: subtransaction_tested_test got arg 'f'
+ pltcl_wrapper
+----------------------------------------
+ ERROR: syntax error at or near "error"
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+(0 rows)
+
+TRUNCATE subtransaction_tbl;
+SELECT pltcl_wrapper('SELECT subtransaction_nested_test(''t'')');
+NOTICE: subtransaction_tested_test got arg 't'
+NOTICE: Swallowed syntax error at or near "error"
+ pltcl_wrapper
+---------------
+ SUCCESS: 1
+(1 row)
+
+SELECT * FROM subtransaction_tbl;
+ i
+---
+ 1
+ 2
+(2 rows)
+
+TRUNCATE subtransaction_tbl;
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 2cf7e66..266d7b9 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -323,6 +323,8 @@ static HeapTuple pltcl_build_tuple_result(Tcl_Interp *interp,
pltcl_call_state *call_state);
static void pltcl_init_tuple_store(pltcl_call_state *call_state);
+static int pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[]);
/*
* Hack to override Tcl's builtin Notifier subsystem. This prevents the
@@ -516,7 +518,8 @@ pltcl_init_interp(pltcl_interp_desc *interp_desc, Oid prolang, bool pltrusted)
pltcl_SPI_execute_plan, NULL, NULL);
Tcl_CreateObjCommand(interp, "spi_lastoid",
pltcl_SPI_lastoid, NULL, NULL);
-
+ Tcl_CreateObjCommand(interp, "subtransaction",
+ pltcl_subtransaction, NULL,NULL);
/************************************************************
* Call the appropriate start_proc, if there is one.
*
@@ -3114,3 +3117,44 @@ pltcl_init_tuple_store(pltcl_call_state *call_state)
CurrentResourceOwner = oldowner;
MemoryContextSwitchTo(oldcxt);
}
+
+/*
+ * pltcl_subtransaction - implements tcl level subtransaction block
+ * Called with exactly one argument - piece of Tcl Code, and executes
+ * this code inside subtransaction.
+ * Rolls subtransaction back if Tcl_EvalEx returns TCL_ERROR
+ */
+static int
+pltcl_subtransaction(ClientData cdata, Tcl_Interp *interp,
+ int objc, Tcl_Obj *const objv[])
+{
+ /* Save resource owner and memory context in the local vars */
+ ResourceOwner oldowner = CurrentResourceOwner;
+ MemoryContext oldcontext = CurrentMemoryContext;
+ int retcode;
+ if (objc != 2)
+ {
+ Tcl_WrongNumArgs(interp,1,objv,"command");
+ return TCL_ERROR;
+ }
+
+ BeginInternalSubTransaction(NULL);
+
+ retcode = Tcl_EvalObjEx(interp, objv[1],0);
+
+ if (retcode == TCL_ERROR)
+ {
+ /* Roollback the subtransaction */
+ RollbackAndReleaseCurrentSubTransaction();
+ }
+ else
+ {
+ /* Commit the subtransaction */
+ ReleaseCurrentSubTransaction();
+ }
+ /* Restore resource owner and memory context
+ In case they were changed inside subtransaction */
+ CurrentResourceOwner = oldowner;
+ MemoryContextSwitchTo(oldcontext);
+ return retcode;
+}
diff --git a/src/pl/tcl/sql/pltcl_subxact.sql b/src/pl/tcl/sql/pltcl_subxact.sql
new file mode 100644
index 0000000..071dbdf
--- /dev/null
+++ b/src/pl/tcl/sql/pltcl_subxact.sql
@@ -0,0 +1,94 @@
+--
+-- Test explicit subtransactions
+--
+
+
+CREATE TABLE subtransaction_tbl (
+ i integer
+);
+
+-- test subtransaction successfully commited
+
+CREATE FUNCTION subtransaction_ctx_success() RETURNS text
+AS $$
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(1)"
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES(2)"
+ }
+$$ LANGUAGE pltcl;
+
+BEGIN;
+INSERT INTO subtransaction_tbl VALUES(0);
+SELECT subtransaction_ctx_success();
+COMMIT;
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+-- Test table to see if transactions get properly rolled back
+
+CREATE FUNCTION subtransaction_ctx_test(what_error text = NULL) RETURNS text
+AS $$
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if {$1 == "SPI"} {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES ('oops')"
+ } elseif { $1 == "Tcl"} {
+ elog ERROR "Tcl error"
+ }
+}
+$$ LANGUAGE pltcl;
+
+--
+-- We use this wrapper to catch errors and return errormsg only,
+-- because values of $::errorinfo variable contain procedure name which
+-- includes OID, which may differ during make installcheck
+--
+CREATE FUNCTION pltcl_wrapper(statement text) RETURNS text
+AS $$
+ if [catch {spi_exec $1} msg] {
+ return "ERROR: $msg"
+ } else {
+ return "SUCCESS: $msg"
+ }
+$$ LANGUAGE pltcl;
+SELECT pltcl_wrapper('SELECT subtransaction_ctx_test()');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+SELECT pltcl_wrapper('SELECT subtransaction_ctx_test(''SPI'')');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+SELECT pltcl_wrapper('SELECT subtransaction_ctx_test(''Tcl'')');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+-- Nested subtransactions
+
+CREATE FUNCTION subtransaction_nested_test(swallow boolean = 'f') RETURNS text
+AS $$
+elog NOTICE "subtransaction_tested_test got arg '$1'"
+spi_exec "INSERT INTO subtransaction_tbl VALUES (1)"
+subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (2)"
+ if [catch {
+ subtransaction {
+ spi_exec "INSERT INTO subtransaction_tbl VALUES (3)"
+ spi_exec "error"
+ }
+ } errormsg] {
+ if {$1 != "t"} {
+ error $errormsg $::errorInfo $::errorCode
+ }
+ elog NOTICE "Swallowed $errormsg"
+ }
+}
+return "ok"
+$$ LANGUAGE pltcl;
+
+SELECT pltcl_wrapper('SELECT subtransaction_nested_test()');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
+
+SELECT pltcl_wrapper('SELECT subtransaction_nested_test(''t'')');
+SELECT * FROM subtransaction_tbl;
+TRUNCATE subtransaction_tbl;
2017-03-10 20:31 GMT+01:00 Victor Wagner <vitus@wagner.pp.ru>:
On Thu, 9 Mar 2017 12:04:31 +0100
Pavel Stehule <pavel.stehule@gmail.com> wrote:Now test demonstrate how errors uncaught on the Tcl level interact
with postgresql error system.you can catch the exception outside and write own message
OK, here is patch with tests which don't depend on function OIDs
They ignore stack trace ($::errorinfo global variable) completely,
and analyze just error message.
all tests passed
I have not any other objections
I'll mark this patch as ready for commiter
Regards
Pavel
Show quoted text
--
Victor Wagner <vitus@wagner.pp.ru>
Pavel Stehule <pavel.stehule@gmail.com> writes:
I'll mark this patch as ready for commiter
I've pushed this after mostly-cosmetic cleanup. One thing I changed
that's not cosmetic is to put
MemoryContextSwitchTo(oldcontext);
after the BeginInternalSubTransaction call. I see there was some
discussion upthread about what to do there, but I think this is the
correct answer because it corresponds to what pltcl_subtrans_begin
does. That means that the execution environment of the called Tcl
fragment will be the same as, eg, the loop_body in spi_exec. I do not
think we want it to be randomly different from those pre-existing cases.
Now, I believe that the MemoryContextSwitchTo in pltcl_subtrans_begin
was probably just cargo-culted in there from similar code in plpgsql.
Since pltcl doesn't rely on CurrentMemoryContext to anywhere near the
same degree that plpgsql does, it's possible that we could drop the
MemoryContextSwitchTo in both places, and just let the called code
fragments run in the subtransaction's memory context. But I'm not
convinced of that, and in any case it ought to be undertaken as a
separate patch.
Some other comments:
* Whitespace still wasn't very much per project conventions.
I fixed that by running it through pgindent.
* The golden rule for code style and placement is "make your patch
look like it was always there". Dropping new code at the tail end
of the file is seldom a good avenue to that. I stuck it after
pltcl_SPI_lastoid, on the grounds that it should be with the other
Tcl command functions and should respect the (mostly) alphabetical
order of those functions. Likewise I adopted a header comment
format in keeping with the existing nearby functions.
* I whacked the SGML docs around pretty thoroughly. That addition
wasn't respecting the style of the surrounding text either as to
markup or indentation, and it had some other issues like syntax
errors in the example functions.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers