Fix transaction leak when DMLs contains a non pltsql language functio…

…n which calls another non tsql function inside try catch (#3217) (#3272) When DMLs are executed outside a user transaction block, babelfish starts an implicit transaction to handle errors inside triggers. This is done irrespective of whether the statement actually fires a trigger. Now if the DML calls a non pltsql function which in calls another non tsql func or procedure inside a try catch, we do not properly decrement the pltsql_sys_func_entry_count variable properly. So the return value of pltsql_support_tsql_transactions() is different at beginning of the DML compared to after the end of the statement. This difference leads to babelfish not commit the implict transaction block. As a fix, store the value of pltsql_support_tsql_transactions() at start of execution of DML and just reuse it for after execution. Issues Resolved: [BABEL-5446] Signed-off-by: Tanzeel Khan [email protected]
babelfish-for-postgresql · Dec 18, 2024 · cf2e36c · cf2e36c
1 parent beb4c85
commit cf2e36c
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 7 deletions.
diff --git a/contrib/babelfishpg_tsql/src/pl_exec.c b/contrib/babelfishpg_tsql/src/pl_exec.c
@@ -1036,9 +1036,10 @@ pltsql_exec_trigger(PLtsql_function *func,
 	PLtsql_rec *rec_new,
 			   *rec_old;
 	HeapTuple	rettup;
+	bool     	support_tsql_trans = pltsql_support_tsql_transactions();
 
 	/* Check if this trigger is called as part of any of postgres' function, procedure or trigger. */
-	if (!pltsql_support_tsql_transactions())
+	if (!support_tsql_trans)
 	{
 		ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -1050,7 +1051,7 @@ pltsql_exec_trigger(PLtsql_function *func,
 	 */
 	pltsql_estate_setup(&estate, func, NULL, NULL);
 
-	if (pltsql_support_tsql_transactions() && !pltsql_disable_txn_in_triggers)
+	if (support_tsql_trans && !pltsql_disable_txn_in_triggers)
 		estate.atomic = false;
 
 	estate.trigdata = trigdata;
@@ -1173,15 +1174,15 @@ pltsql_exec_trigger(PLtsql_function *func,
 		 * TSQL triggers terminate if there is no transaction active at the
 		 * end
 		 */
-		if (pltsql_support_tsql_transactions() && !pltsql_disable_txn_in_triggers && NestedTranCount == 0)
+		if (support_tsql_trans && !pltsql_disable_txn_in_triggers && NestedTranCount == 0)
 			ereport(ERROR,
 					(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
 					 errmsg("The transaction ended in the trigger. The batch has been aborted.")));
 
 		/*
 		 * If an error was encountered when executing trigger.
 		 */
-		if (pltsql_support_tsql_transactions() && !pltsql_disable_txn_in_triggers && exec_state_call_stack->error_data.trigger_error)
+		if (support_tsql_trans && !pltsql_disable_txn_in_triggers && exec_state_call_stack->error_data.trigger_error)
 			ereport(ERROR,
 					(errcode(ERRCODE_TRIGGERED_ACTION_EXCEPTION),
 					 errmsg("An error was raised during trigger execution. The batch has been aborted and the user transaction, if any, has been rolled back.")));
@@ -4622,6 +4623,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 	bool		fmtonly_enabled = true;
 	CmdType		cmd = CMD_UNKNOWN;
 	bool		enable_txn_in_triggers = !pltsql_disable_txn_in_triggers;
+	bool		support_tsql_trans = pltsql_support_tsql_transactions();
 	StringInfoData query;
 	bool		need_path_reset = false;
 	char	   *cur_dbname = get_cur_db_name();
@@ -4799,7 +4801,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 			/* Open nesting level in engine */
 			BeginCompositeTriggers(CurrentMemoryContext);
 			/* TSQL commands must run inside an explicit transaction */
-			if (!pltsql_disable_batch_auto_commit && pltsql_support_tsql_transactions() &&
+			if (!pltsql_disable_batch_auto_commit && support_tsql_trans &&
 				stmt->txn_data == NULL && !IsTransactionBlockActive())
 			{
 				MemoryContext oldCxt = CurrentMemoryContext;
@@ -4825,7 +4827,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 										  portal, expr, cmd, paramLI);
 		else if (stmt->need_to_push_result)
 			rc = execute_plan_and_push_result(estate, expr, paramLI);
-		else if (stmt->txn_data != NULL && !pltsql_support_tsql_transactions())
+		else if (stmt->txn_data != NULL && !support_tsql_trans)
 		{
 			elog(DEBUG2, "TSQL TXN Execute transaction command with PG semantics");
 			rc = execute_txn_command(estate, stmt);
@@ -5058,7 +5060,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 		 */
 		/* TODO To let procedure call from PSQL work with old semantics */
 		if ((!pltsql_disable_batch_auto_commit || (stmt->txn_data != NULL)) &&
-			pltsql_support_tsql_transactions() &&
+			support_tsql_trans &&
 			(enable_txn_in_triggers || estate->trigdata == NULL) &&
 			!ro_func && !estate->insert_exec)
 		{

diff --git a/test/JDBC/expected/TestBasicInterop.out b/test/JDBC/expected/TestBasicInterop.out
@@ -186,3 +186,31 @@ GO
 -- psql
 DROP PROCEDURE pg_interop_proc_sp_exec
 GO
+
+-- tsql
+CREATE TABLE babel_5446 (Test INT NULL)
+GO
+INSERT INTO babel_5446 SELECT ISNUMERIC('test') AS my_isnumeric
+GO
+~~ROW COUNT: 1~~
+
+SELECT @@trancount
+GO
+~~START~~
+int
+0
+~~END~~
+
+
+INSERT INTO babel_5446 SELECT ISNUMERIC('test') AS my_isnumeric
+SELECT @@trancount
+GO
+~~ROW COUNT: 1~~
+
+~~START~~
+int
+0
+~~END~~
+
+DROP TABLE babel_5446
+GO
diff --git a/test/JDBC/input/interoperability/TestBasicInterop.mix b/test/JDBC/input/interoperability/TestBasicInterop.mix
@@ -127,3 +127,17 @@ GO
 -- psql
 DROP PROCEDURE pg_interop_proc_sp_exec
 GO
+
+-- tsql
+CREATE TABLE babel_5446 (Test INT NULL)
+GO
+INSERT INTO babel_5446 SELECT ISNUMERIC('test') AS my_isnumeric
+GO
+SELECT @@trancount
+GO
+
+INSERT INTO babel_5446 SELECT ISNUMERIC('test') AS my_isnumeric
+SELECT @@trancount
+GO
+DROP TABLE babel_5446
+GO