From cf2e36ce9fbedc242a00f3ab8b71bd57da972ca6 Mon Sep 17 00:00:00 2001 From: Tanzeel Khan <140405735+tanscorpio7@users.noreply.github.com> Date: Wed, 18 Dec 2024 16:23:48 +0530 Subject: [PATCH] Fix transaction leak when DMLs contains a non pltsql language function 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 tzlkhan@amazon.com --- contrib/babelfishpg_tsql/src/pl_exec.c | 16 ++++++----- test/JDBC/expected/TestBasicInterop.out | 28 +++++++++++++++++++ .../interoperability/TestBasicInterop.mix | 14 ++++++++++ 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/contrib/babelfishpg_tsql/src/pl_exec.c b/contrib/babelfishpg_tsql/src/pl_exec.c index 49126ada78..81a4a451b7 100644 --- 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,7 +1174,7 @@ 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."))); @@ -1181,7 +1182,7 @@ pltsql_exec_trigger(PLtsql_function *func, /* * 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 index 55a1fe3a25..f5ce27a05b 100644 --- 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 index 589497d7cd..592f191182 100644 --- 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