Skip to content

Commit 20c3898

Browse files
rophyrophy
committed
fix: handle econtext stack in multi-statement queries with COMMIT/ROLLBACK
In multi-statement queries, the transaction block state is TBLOCK_IMPLICIT_INPROGRESS. When SPI_commit() is called, CommitTransactionCommand() only does CommandCounterIncrement() instead of CommitTransaction(), so xact callbacks are not fired. This left stale entries on simple_econtext_stack causing crashes. Clear the econtext stack explicitly in exec_stmt_commit() and exec_stmt_rollback() to handle both single and multi-statement query contexts uniformly. Add regression tests for nested procedure calls with COMMIT/ROLLBACK in multi-statement queries.
1 parent 307b066 commit 20c3898

3 files changed

Lines changed: 399 additions & 0 deletions

File tree

‎src/pl/plisql/src/expected/plisql_transaction.out‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,233 @@ SELECT * FROM test1;
745745
2 |
746746
(2 rows)
747747

748+
-- Test nested procedure calls with COMMIT (Issue #1007)
749+
-- These tests verify that COMMIT works correctly in nested procedure calls,
750+
-- including in multi-statement query contexts.
751+
CREATE TABLE test_nested_commit (id int);
752+
-- Inner procedure with COMMIT
753+
CREATE PROCEDURE nested_inner_commit()
754+
LANGUAGE plisql
755+
SECURITY INVOKER
756+
AS $$
757+
BEGIN
758+
INSERT INTO test_nested_commit VALUES (1);
759+
COMMIT;
760+
INSERT INTO test_nested_commit VALUES (2);
761+
END;
762+
$$;
763+
/
764+
-- Outer procedure calling inner
765+
CREATE PROCEDURE nested_outer_commit()
766+
LANGUAGE plisql
767+
SECURITY INVOKER
768+
AS $$
769+
BEGIN
770+
INSERT INTO test_nested_commit VALUES (0);
771+
CALL nested_inner_commit();
772+
INSERT INTO test_nested_commit VALUES (3);
773+
END;
774+
$$;
775+
/
776+
-- Test 1: Basic nested call with COMMIT
777+
TRUNCATE test_nested_commit;
778+
CALL nested_outer_commit();
779+
SELECT * FROM test_nested_commit ORDER BY id;
780+
id
781+
----
782+
0
783+
1
784+
2
785+
3
786+
(4 rows)
787+
788+
-- Test 2: Multi-statement query with nested CALL (previously crashed)
789+
TRUNCATE test_nested_commit;
790+
SELECT 1 AS setup; CALL nested_outer_commit(); SELECT 2 AS done;
791+
setup
792+
-------
793+
1
794+
(1 row)
795+
796+
done
797+
------
798+
2
799+
(1 row)
800+
801+
SELECT * FROM test_nested_commit ORDER BY id;
802+
id
803+
----
804+
0
805+
1
806+
2
807+
3
808+
(4 rows)
809+
810+
-- Test 3: Oracle-style call (without CALL keyword) in nested procedure
811+
CREATE PROCEDURE nested_outer_oracle_style()
812+
LANGUAGE plisql
813+
SECURITY INVOKER
814+
AS $$
815+
BEGIN
816+
INSERT INTO test_nested_commit VALUES (10);
817+
nested_inner_commit(); -- Oracle-style call
818+
INSERT INTO test_nested_commit VALUES (13);
819+
END;
820+
$$;
821+
/
822+
TRUNCATE test_nested_commit;
823+
CALL nested_outer_oracle_style();
824+
SELECT * FROM test_nested_commit ORDER BY id;
825+
id
826+
----
827+
1
828+
2
829+
10
830+
13
831+
(4 rows)
832+
833+
-- Test 4: Multi-statement with Oracle-style nested call
834+
TRUNCATE test_nested_commit;
835+
SELECT 'before' AS stage; CALL nested_outer_oracle_style(); SELECT 'after' AS stage;
836+
stage
837+
--------
838+
before
839+
(1 row)
840+
841+
stage
842+
-------
843+
after
844+
(1 row)
845+
846+
SELECT * FROM test_nested_commit ORDER BY id;
847+
id
848+
----
849+
1
850+
2
851+
10
852+
13
853+
(4 rows)
854+
855+
-- Test 5: Deeply nested calls (3 levels)
856+
CREATE PROCEDURE nested_level3()
857+
LANGUAGE plisql
858+
SECURITY INVOKER
859+
AS $$
860+
BEGIN
861+
INSERT INTO test_nested_commit VALUES (103);
862+
COMMIT;
863+
END;
864+
$$;
865+
/
866+
CREATE PROCEDURE nested_level2()
867+
LANGUAGE plisql
868+
SECURITY INVOKER
869+
AS $$
870+
BEGIN
871+
INSERT INTO test_nested_commit VALUES (102);
872+
CALL nested_level3();
873+
INSERT INTO test_nested_commit VALUES (104);
874+
END;
875+
$$;
876+
/
877+
CREATE PROCEDURE nested_level1()
878+
LANGUAGE plisql
879+
SECURITY INVOKER
880+
AS $$
881+
BEGIN
882+
INSERT INTO test_nested_commit VALUES (101);
883+
CALL nested_level2();
884+
INSERT INTO test_nested_commit VALUES (105);
885+
END;
886+
$$;
887+
/
888+
TRUNCATE test_nested_commit;
889+
CALL nested_level1();
890+
SELECT * FROM test_nested_commit ORDER BY id;
891+
id
892+
-----
893+
101
894+
102
895+
103
896+
104
897+
105
898+
(5 rows)
899+
900+
-- Test 6: Multi-statement with deeply nested calls
901+
TRUNCATE test_nested_commit;
902+
SELECT 'start'; CALL nested_level1(); SELECT 'end';
903+
?column?
904+
----------
905+
start
906+
(1 row)
907+
908+
?column?
909+
----------
910+
end
911+
(1 row)
912+
913+
SELECT * FROM test_nested_commit ORDER BY id;
914+
id
915+
-----
916+
101
917+
102
918+
103
919+
104
920+
105
921+
(5 rows)
922+
923+
-- Test 7: ROLLBACK in nested procedure
924+
CREATE PROCEDURE nested_inner_rollback()
925+
LANGUAGE plisql
926+
SECURITY INVOKER
927+
AS $$
928+
BEGIN
929+
INSERT INTO test_nested_commit VALUES (201);
930+
ROLLBACK;
931+
INSERT INTO test_nested_commit VALUES (202);
932+
END;
933+
$$;
934+
/
935+
CREATE PROCEDURE nested_outer_rollback()
936+
LANGUAGE plisql
937+
SECURITY INVOKER
938+
AS $$
939+
BEGIN
940+
INSERT INTO test_nested_commit VALUES (200);
941+
CALL nested_inner_rollback();
942+
INSERT INTO test_nested_commit VALUES (203);
943+
END;
944+
$$;
945+
/
946+
TRUNCATE test_nested_commit;
947+
SELECT 'pre'; CALL nested_outer_rollback(); SELECT 'post';
948+
?column?
949+
----------
950+
pre
951+
(1 row)
952+
953+
?column?
954+
----------
955+
post
956+
(1 row)
957+
958+
SELECT * FROM test_nested_commit ORDER BY id;
959+
id
960+
-----
961+
202
962+
203
963+
(2 rows)
964+
965+
-- Clean up nested commit tests
966+
DROP PROCEDURE nested_inner_commit();
967+
DROP PROCEDURE nested_outer_commit();
968+
DROP PROCEDURE nested_outer_oracle_style();
969+
DROP PROCEDURE nested_level1();
970+
DROP PROCEDURE nested_level2();
971+
DROP PROCEDURE nested_level3();
972+
DROP PROCEDURE nested_inner_rollback();
973+
DROP PROCEDURE nested_outer_rollback();
974+
DROP TABLE test_nested_commit;
748975
DROP TABLE test1;
749976
DROP TABLE test2;
750977
DROP TABLE test3;

‎src/pl/plisql/src/pl_exec.c‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5318,7 +5318,18 @@ exec_stmt_commit(PLiSQL_execstate * estate, PLiSQL_stmt_commit * stmt)
53185318
/*
53195319
* We need to build new simple-expression infrastructure, since the old
53205320
* data structures are gone.
5321+
*
5322+
* In some execution contexts (e.g., multi-statement queries with implicit
5323+
* transaction blocks), the transaction callbacks may not be fired even
5324+
* though SPI_commit succeeded. In such cases, the econtext stack would
5325+
* retain stale entries pointing to freed memory. Clear the stack here to
5326+
* handle both cases uniformly - if the callback already cleared it, this
5327+
* is harmless; if not, this prevents stack corruption.
53215328
*/
5329+
simple_econtext_stack = NULL;
5330+
shared_simple_eval_estate = NULL;
5331+
shared_simple_eval_resowner = NULL;
5332+
53225333
estate->simple_eval_estate = NULL;
53235334
estate->simple_eval_resowner = NULL;
53245335
plisql_create_econtext(estate);
@@ -5342,7 +5353,18 @@ exec_stmt_rollback(PLiSQL_execstate * estate, PLiSQL_stmt_rollback * stmt)
53425353
/*
53435354
* We need to build new simple-expression infrastructure, since the old
53445355
* data structures are gone.
5356+
*
5357+
* In some execution contexts (e.g., multi-statement queries with implicit
5358+
* transaction blocks), the transaction callbacks may not be fired even
5359+
* though SPI_rollback succeeded. In such cases, the econtext stack would
5360+
* retain stale entries pointing to freed memory. Clear the stack here to
5361+
* handle both cases uniformly - if the callback already cleared it, this
5362+
* is harmless; if not, this prevents stack corruption.
53455363
*/
5364+
simple_econtext_stack = NULL;
5365+
shared_simple_eval_estate = NULL;
5366+
shared_simple_eval_resowner = NULL;
5367+
53465368
estate->simple_eval_estate = NULL;
53475369
estate->simple_eval_resowner = NULL;
53485370
plisql_create_econtext(estate);

0 commit comments

Comments
 (0)