Fix error when OUTPUT INTO table variable statement fires trigger (#2639

) (#2796)

Fix error portal snapshots (1) did not account for all active snapshots (2) when OUTPUT INTO table variable statement fires triggers.

pltsql statements that could fire trigger must always run inside a transaction block. If a txn block is not active, we create one. We exclude some types of statements from this, one of them being DML on table variables since they can never fire a trigger. We detect this using the variable mod_stmt_tablevar.

Now an edge case here is OUTPUT into table variable stmt. Which we internally transform into two DML nodes.
TOP INSERT NODE (signifies the output into clause)
Another DELETE / UPDATE / INSERT node inside the top insert node -> with clause (specifies the first part of the output into clause )
As a fix we simply skip transaction commands inside pltsql UDFs and not rely on mod_stmt_tablevar on anymore.

Also blocked certain cases of OUTPUT clause which should not be allowed inside pltsql UDFs.

    OUTPUT clause flushes results to client (INTO clause is missing)
    OUTPUT INTO clause has a non local object as its target. (UDFs only allow DML on local objects)

Issues Resolved: BABEL-4859

Signed-off-by: Tanzeel Khan [email protected]

contrib/babelfishpg_tsql/src/pl_exec.c

@@ -4608,6 +4608,9 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 
 	/* fetch current search_path */
 	char	   *old_search_path = NULL;
+	bool		ro_func = (estate->func->fn_prokind == PROKIND_FUNCTION) &&
+						  (estate->func->fn_is_trigger == PLTSQL_NOT_TRIGGER) &&
+						  (strcmp(estate->func->fn_signature, "inline_code_block") != 0);
 
 	if (stmt->original_query)
 		original_query_string = stmt->original_query;
@@ -4762,7 +4765,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 		 * tsql_select_assign_stmt (select @a=1). with ANTLR=off, it is
 		 * handled in PLtsql_stmt_query_set.
 		 */
-		if (stmt->need_to_push_result || stmt->is_tsql_select_assign_stmt || stmt->mod_stmt_tablevar)
+		if (stmt->need_to_push_result || stmt->is_tsql_select_assign_stmt || ro_func)
 			enable_txn_in_triggers = false;
 
 		if (enable_txn_in_triggers)
@@ -5031,10 +5034,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 		if ((!pltsql_disable_batch_auto_commit || (stmt->txn_data != NULL)) &&
 			pltsql_support_tsql_transactions() &&
 			(enable_txn_in_triggers || estate->trigdata == NULL) &&
-			!(estate->func->fn_prokind == PROKIND_FUNCTION &&
-			  estate->func->fn_is_trigger == PLTSQL_NOT_TRIGGER &&
-			  strcmp(estate->func->fn_signature, "inline_code_block") != 0) &&
-			!estate->insert_exec)
+			!ro_func && !estate->insert_exec)
 		{
 			commit_stmt(estate, (estate->tsql_trigger_flags & TSQL_TRAN_STARTED));
 

contrib/babelfishpg_tsql/src/tsqlIface.cpp

@@ -1420,6 +1420,23 @@ class tsqlBuilder : public tsqlCommonMutator
 			else if (ctx->delete_statement() && ctx->delete_statement()->delete_statement_from()->ddl_object() && !ctx->delete_statement()->delete_statement_from()->ddl_object()->local_id()  &&
 					(ctx->delete_statement()->table_sources() ? ::getFullText(ctx->delete_statement()->table_sources()).c_str()[0] != '@' : true)) /* delete non-local object, table variables are allowed */
 				throw PGErrorWrapperException(ERROR, ERRCODE_INVALID_FUNCTION_DEFINITION, "'DELETE' cannot be used within a function", getLineAndPos(ctx->delete_statement()->delete_statement_from()->ddl_object()));
+
+			/*
+			 * Reject if OUTPUT clause is missing INTO (returning to client) or OUTPUT INTO non local object
+			 */
+
+			if (ctx->insert_statement() && ctx->insert_statement()->output_clause() && (!ctx->insert_statement()->output_clause()->INTO() || !ctx->insert_statement()->output_clause()->LOCAL_ID()))
+			{
+				throw PGErrorWrapperException(ERROR, ERRCODE_INVALID_FUNCTION_DEFINITION, "Invalid use of a side-effecting operator 'INSERT' within a function.", getLineAndPos(ctx->insert_statement()->output_clause()));
+			}
+			else if (ctx->update_statement() && ctx->update_statement()->output_clause() && (!ctx->update_statement()->output_clause()->INTO() || !ctx->update_statement()->output_clause()->LOCAL_ID()))
+			{
+				throw PGErrorWrapperException(ERROR, ERRCODE_INVALID_FUNCTION_DEFINITION, "Invalid use of a side-effecting operator 'UPDATE' within a function.", getLineAndPos(ctx->update_statement()->output_clause()));
+			}
+			else if (ctx->delete_statement() && ctx->delete_statement()->output_clause() && (!ctx->delete_statement()->output_clause()->INTO() || !ctx->delete_statement()->output_clause()->LOCAL_ID()))
+			{
+				throw PGErrorWrapperException(ERROR, ERRCODE_INVALID_FUNCTION_DEFINITION, "Invalid use of a side-effecting operator 'DELETE' within a function.", getLineAndPos(ctx->delete_statement()->output_clause()));
+			}
 		}
 
 		/* we must add previous rewrite at first. */