INSERT EXEC: Code cleanup and improvements

- Remove dead code: flush_temp_table_to_target() function (unused, replaced by
  flush_insert_exec_temp_table())
- Remove dead estate fields from PLtsql_execstate: insert_exec_temp_table_oid,
  insert_exec_target_table, insert_exec_column_list (new design uses global
  variables instead)
- Remove no-op functions: pltsql_insert_exec_enter_trycatch() and
  pltsql_insert_exec_exit_trycatch() (TRY-CATCH detection now uses err_ctx_stack
  via pltsql_insert_exec_in_trycatch())
- Change elog(LOG, ...) to elog(DEBUG1, ...) in TRY-CATCH cleanup code to avoid
  printing to server log in production
- Improve DROP TABLE check in hooks.c: use OID comparison for regular tables
  (distinguishes schema-qualified names like schema1.orders vs schema2.orders),
  fall back to name comparison for temp tables

Author: monk0707

contrib/babelfishpg_tsql/src/hooks.c

@@ -824,59 +824,65 @@ pltsql_bbfCustomProcessUtility(ParseState *pstate, PlannedStmt *pstmt, const cha
 		}
 		case T_DropStmt:
 		{
-			/*
-			 * Check if DROP TABLE is trying to drop the INSERT EXEC target table.
-			 * SQL Server error 556: "cannot DROP TABLE because it is being used 
-			 * by active queries in this session"
-			 */
-			DropStmt *stmt = (DropStmt *) parsetree;
-			
-			if (stmt->removeType == OBJECT_TABLE && pltsql_insert_exec_active())
+			DropStmt *drop = (DropStmt *) parsetree;
+			if (drop->removeType == OBJECT_TABLE && pltsql_insert_exec_active())
 			{
-				const char *target_table = pltsql_get_insert_exec_target_table();
-				
-				if (target_table != NULL)
+				Oid target_oid = pltsql_get_insert_exec_target_rel_oid();
+
+				if (OidIsValid(target_oid))
 				{
-					ListCell *cell;
-					
-					foreach(cell, stmt->objects)
+					/*
+					 * Regular table: use OID comparison so schema-qualified names
+					 * like schema1.orders and schema2.orders are distinguished.
+					 */
+					ListCell *lc;
+					foreach(lc, drop->objects)
 					{
-						List *names = (List *) lfirst(cell);
-						char *table_name = NULL;
-						
-						/* Get the table name from the list (last element) */
-						if (list_length(names) > 0)
-						{
-							table_name = strVal(llast(names));
-						}
-						
-						if (table_name != NULL)
+						List     *names    = lfirst_node(List, lc);
+						RangeVar *rv       = makeRangeVarFromNameList(names);
+						Oid       drop_oid = RangeVarGetRelid(rv, NoLock, true /* missing_ok */);
+
+						if (OidIsValid(drop_oid) && drop_oid == target_oid)
+							ereport(ERROR,
+									(errcode(ERRCODE_OBJECT_IN_USE),
+									 errmsg("cannot %s \"%s\" because it is "
+											"being used by active queries in "
+											"this session",
+											"DROP TABLE", get_rel_name(drop_oid))));
+					}
+				}
+				else
+				{
+					/*
+					 * Temp table / table variable: session-local, no persistent OID.
+					 * Fall back to unqualified name comparison (safe over-block).
+					 */
+					const char *target      = pltsql_get_insert_exec_target_table();
+					const char *dot         = target ? strrchr(target, '.') : NULL;
+					const char *unqualified = dot ? (dot + 1) : target;
+					ListCell   *lc;
+
+					if (unqualified != NULL)
+					{
+						foreach(lc, drop->objects)
 						{
-							/*
-							 * Compare table names (case-insensitive for temp tables).
-							 * The target_table might include schema prefix, so we need
-							 * to compare just the table name part.
-							 */
-							const char *target_name = strrchr(target_table, '.');
-							if (target_name != NULL)
-								target_name++;  /* Skip the dot */
-							else
-								target_name = target_table;
-							
-							if (pg_strcasecmp(table_name, target_name) == 0)
-							{
+							List *names   = lfirst_node(List, lc);
+							char *relname = strVal(llast(names));
+
+							if (pg_strcasecmp(relname, unqualified) == 0)
 								ereport(ERROR,
 										(errcode(ERRCODE_OBJECT_IN_USE),
-										 errmsg("cannot %s \"%s\" because it is being used by active queries in this session",
-												"DROP TABLE", table_name)));
-							}
+										 errmsg("cannot %s \"%s\" because it is "
+												"being used by active queries in "
+												"this session",
+												"DROP TABLE", relname)));
 						}
 					}
 				}
 			}
-			/* Let the default handler process the DROP */
 			break;
 		}
+
 		default:
 			return false;
 			break;

contrib/babelfishpg_tsql/src/pl_exec-2.c

@@ -711,34 +711,23 @@ pltsql_get_insert_exec_target_table(void)
 }
 
 /*
- * Get the column list for INSERT EXEC.
- */
-const char *
-pltsql_get_insert_exec_column_list(void)
-{
-	return insert_exec_column_list;
-}
-
-/*
- * Increment TRY-CATCH depth when entering a TRY block during INSERT EXEC.
- * NOTE: This function is now a no-op. TRY-CATCH detection is done via err_ctx_stack.
- * Kept for API compatibility but does nothing.
+ * Get the target table OID for INSERT EXEC.
+ * This is the OID of the actual target table (not the temp table).
+ * Used for schema-qualified DROP TABLE checks.
  */
-void
-pltsql_insert_exec_enter_trycatch(void)
+Oid
+pltsql_get_insert_exec_target_rel_oid(void)
 {
-	/* No-op: TRY-CATCH detection is done via err_ctx_stack in pltsql_insert_exec_in_trycatch() */
+	return insert_exec_target_rel_oid;
 }
 
 /*
- * Decrement TRY-CATCH depth when exiting a TRY block during INSERT EXEC.
- * NOTE: This function is now a no-op. TRY-CATCH detection is done via err_ctx_stack.
- * Kept for API compatibility but does nothing.
+ * Get the column list for INSERT EXEC.
  */
-void
-pltsql_insert_exec_exit_trycatch(void)
+const char *
+pltsql_get_insert_exec_column_list(void)
 {
-	/* No-op: TRY-CATCH detection is done via err_ctx_stack in pltsql_insert_exec_in_trycatch() */
+	return insert_exec_column_list;
 }
 
 /*
@@ -2088,137 +2077,6 @@ flush_insert_exec_temp_table(PLtsql_execstate *estate)
 		pfree(saved_column_list);
 }
 
-/*
- * Flush all rows from the temp table to the target table.
- * This is the final step of INSERT EXEC - move data from buffer to target.
- * When no column list is specified, we need to explicitly list non-IDENTITY
- * columns to avoid column count mismatch.
- */
-void
-flush_temp_table_to_target(PLtsql_execstate *estate)
-{
-	char			temp_table_name[NAMEDATALEN];
-	StringInfoData	flush_query;
-	int				rc;
-
-	if (!OidIsValid(estate->insert_exec_temp_table_oid) ||
-		estate->insert_exec_target_table == NULL)
-	{
-		return;
-	}
-
-	snprintf(temp_table_name, sizeof(temp_table_name),
-			 "#insert_exec_buf_%d", MyProcPid);
-
-	initStringInfo(&flush_query);
-	
-	if (estate->insert_exec_column_list != NULL)
-	{
-		/* User specified columns - use them directly */
-		appendStringInfo(&flush_query,
-			"INSERT INTO %s (%s) SELECT * FROM %s",
-			estate->insert_exec_target_table,
-			estate->insert_exec_column_list,
-			temp_table_name);
-	}
-	else
-	{
-		/*
-		 * No column list specified - we need to build one excluding
-		 * IDENTITY and computed columns to match the temp table structure.
-		 */
-		StringInfoData col_query;
-		StringInfoData non_identity_cols;
-		bool		first_col = true;
-		int			proc_count;
-		uint64		i;
-		
-		initStringInfo(&col_query);
-		initStringInfo(&non_identity_cols);
-		
-		appendStringInfo(&col_query,
-			"SELECT a.attname "
-			"FROM pg_attribute a "
-			"JOIN pg_class c ON a.attrelid = c.oid "
-			"WHERE c.relname = '%s' "
-			"AND a.attnum > 0 "
-			"AND NOT a.attisdropped "
-			"AND a.attidentity = '' "
-			"AND a.attgenerated = '' "
-			"ORDER BY a.attnum",
-			estate->insert_exec_target_table);
-		
-		rc = SPI_execute(col_query.data, true, 0);
-		if (rc != SPI_OK_SELECT)
-		{
-			pfree(col_query.data);
-			pfree(non_identity_cols.data);
-			/* Fall back to simple INSERT */
-			appendStringInfo(&flush_query,
-				"INSERT INTO %s SELECT * FROM %s",
-				estate->insert_exec_target_table,
-				temp_table_name);
-		}
-		else
-		{
-			proc_count = SPI_processed;
-			
-			if (proc_count == 0)
-			{
-				/* No columns found, fall back */
-				pfree(col_query.data);
-				pfree(non_identity_cols.data);
-				appendStringInfo(&flush_query,
-					"INSERT INTO %s SELECT * FROM %s",
-					estate->insert_exec_target_table,
-					temp_table_name);
-			}
-			else
-			{
-				/* Build column list */
-				for (i = 0; i < proc_count; i++)
-				{
-					char *colname = SPI_getvalue(SPI_tuptable->vals[i], 
-												 SPI_tuptable->tupdesc, 1);
-					if (colname != NULL)
-					{
-						if (!first_col)
-							appendStringInfoString(&non_identity_cols, ", ");
-						appendStringInfoString(&non_identity_cols, colname);
-						first_col = false;
-					}
-				}
-				
-				SPI_freetuptable(SPI_tuptable);
-				pfree(col_query.data);
-				
-				appendStringInfo(&flush_query,
-					"INSERT INTO %s (%s) SELECT * FROM %s",
-					estate->insert_exec_target_table,
-					non_identity_cols.data,
-					temp_table_name);
-				
-				pfree(non_identity_cols.data);
-			}
-		}
-	}
-
-	elog(DEBUG1, "INSERT-EXEC: Flushing temp table to target: %s", flush_query.data);
-
-	rc = SPI_execute(flush_query.data, false, 0);
-	if (rc != SPI_OK_INSERT)
-		elog(ERROR, "INSERT-EXEC: Failed to flush temp table to target: %s",
-			 SPI_result_code_string(rc));
-
-	/* Update rowcount */
-	estate->eval_processed = SPI_processed;
-	exec_set_rowcount(SPI_processed);
-	exec_set_found(estate, SPI_processed > 0);
-
-	pfree(flush_query.data);
-	SPI_freetuptable(SPI_tuptable);
-}
-
 /* helper function to get current T-SQL estate */
 PLtsql_execstate *get_current_tsql_estate(void);
 PLtsql_execstate *get_outermost_tsql_estate(int *nestlevel);
@@ -2758,12 +2616,6 @@ exec_stmt_try_catch(PLtsql_execstate *estate, PLtsql_stmt_try_catch *stmt)
 
 	MemoryContext stmt_mcontext;
 
-	/*
-	 * Track TRY-CATCH depth for INSERT EXEC.
-	 * This is used as a fallback when exec_state_call_stack is NULL.
-	 */
-	pltsql_insert_exec_enter_trycatch();
-
 	/*
 	 * Check if INSERT EXEC is active at the start of the TRY block.
 	 * If so, we need special handling to preserve INSERT EXEC data
@@ -2897,7 +2749,7 @@ exec_stmt_try_catch(PLtsql_execstate *estate, PLtsql_stmt_try_catch *stmt)
 		MemoryContextSwitchTo(oldcontext);
 		CurrentResourceOwner = oldowner;
 
-		elog(LOG, "INSERT-EXEC TRY-CATCH cleanup: checking if active, is_stmt_terminating=%d, insert_exec_was_active=%d",
+		elog(DEBUG1, "INSERT-EXEC TRY-CATCH cleanup: checking if active, is_stmt_terminating=%d, insert_exec_was_active=%d",
 			 is_stmt_terminating, insert_exec_was_active);
 
 		/*
@@ -2915,7 +2767,7 @@ exec_stmt_try_catch(PLtsql_execstate *estate, PLtsql_stmt_try_catch *stmt)
 		{
 			Oid temp_oid = pltsql_get_insert_exec_temp_table_oid();
 
-			elog(LOG, "INSERT-EXEC TRY-CATCH cleanup: is_stmt_terminating=%d, insert_exec_was_active=%d, temp_oid=%u",
+			elog(DEBUG1, "INSERT-EXEC TRY-CATCH cleanup: is_stmt_terminating=%d, insert_exec_was_active=%d, temp_oid=%u",
 				 is_stmt_terminating, insert_exec_was_active, temp_oid);
 
 			/* Close target table and release lock first */
@@ -2930,17 +2782,17 @@ exec_stmt_try_catch(PLtsql_execstate *estate, PLtsql_stmt_try_catch *stmt)
 			 */
 			if (is_stmt_terminating && insert_exec_was_active && OidIsValid(temp_oid))
 			{
-				elog(LOG, "INSERT-EXEC TRY-CATCH cleanup: dropping temp table");
+				elog(DEBUG1, "INSERT-EXEC TRY-CATCH cleanup: dropping temp table");
 				drop_insert_exec_temp_table(temp_oid);
 			}
 		}
 		else if (pltsql_insert_exec_active())
 		{
-			elog(LOG, "INSERT-EXEC TRY-CATCH cleanup: INSERT EXEC active but TRY-CATCH is inside procedure, not cleaning up");
+			elog(DEBUG1, "INSERT-EXEC TRY-CATCH cleanup: INSERT EXEC active but TRY-CATCH is inside procedure, not cleaning up");
 		}
 		else
 		{
-			elog(LOG, "INSERT-EXEC TRY-CATCH cleanup: INSERT EXEC not active");
+			elog(DEBUG1, "INSERT-EXEC TRY-CATCH cleanup: INSERT EXEC not active");
 		}
 
 		/*
@@ -2994,11 +2846,6 @@ exec_stmt_try_catch(PLtsql_execstate *estate, PLtsql_stmt_try_catch *stmt)
 
 	Assert(save_cur_error == estate->cur_error->error);
 
-	/*
-	 * Decrement TRY-CATCH depth for INSERT EXEC tracking.
-	 */
-	pltsql_insert_exec_exit_trycatch();
-
 	estate->err_text = NULL;
 
 	/*

contrib/babelfishpg_tsql/src/pltsql.h

@@ -1545,11 +1545,6 @@ typedef struct PLtsql_execstate
 	 */
 	bool		insert_exec;
 
-	/* INSERT EXEC temp table buffering fields */
-	Oid			insert_exec_temp_table_oid;		/* OID of temp table for buffering */
-	char	   *insert_exec_target_table;		/* Final target table name */
-	char	   *insert_exec_column_list;		/* Column list for final INSERT (NULL = all) */
-
 	List	   *explain_infos;
 	instr_time	planning_start;
 	instr_time	planning_end;
@@ -2409,9 +2404,8 @@ extern bool pltsql_insert_exec_flush_in_progress(void);
 extern int pltsql_get_insert_exec_base_tran_count(void);
 extern Oid pltsql_get_insert_exec_temp_table_oid(void);
 extern const char *pltsql_get_insert_exec_target_table(void);
+extern Oid pltsql_get_insert_exec_target_rel_oid(void);
 extern const char *pltsql_get_insert_exec_column_list(void);
-extern void pltsql_insert_exec_enter_trycatch(void);
-extern void pltsql_insert_exec_exit_trycatch(void);
 extern bool pltsql_insert_exec_in_trycatch(void);
 extern bool pltsql_insert_exec_should_cleanup_on_trycatch(void);
 extern Oid pltsql_get_and_clear_insert_exec_temp_table_for_cleanup(void);
@@ -2426,7 +2420,6 @@ extern bool pltsql_insert_exec_verify_schema(void);
 extern DestReceiver *CreateInsertExecDestReceiver(Oid temp_table_oid);
 extern Oid create_insert_exec_temp_table(const char *target_table, const char *column_list);
 extern void drop_insert_exec_temp_table(Oid temp_table_oid);
-extern void flush_temp_table_to_target(PLtsql_execstate *estate);
 extern void flush_insert_exec_temp_table(PLtsql_execstate *estate);
 
 #define NUM_DB_OBJECTS 11