mirror of
				https://github.com/postgres/postgres.git
				synced 2025-10-29 22:49:41 +03:00 
			
		
		
		
	Fix error-cleanup mistakes in exec_stmt_call().
Commit 15c729347 was a couple bricks shy of a load: we need to
ensure that expr->plan gets reset to NULL on any error exit,
if it's not supposed to be saved.  Also ensure that the
stmt->target calculation gets redone if needed.
The easy way to exhibit a problem is to set up code that
violates the writable-argument restriction and then execute
it twice.  But error exits out of, eg, setup_param_list()
could also break it.  Make the existing PG_TRY block cover
all of that code to be sure.
Per report from Pavel Stehule.
Discussion: https://postgr.es/m/CAFj8pRAeXNTO43W2Y0Cn0YOVFPv1WpYyOqQrrzUiN6s=dn7gCg@mail.gmail.com
			
			
This commit is contained in:
		| @@ -2072,39 +2072,50 @@ static int | |||||||
| exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) | exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) | ||||||
| { | { | ||||||
| 	PLpgSQL_expr *expr = stmt->expr; | 	PLpgSQL_expr *expr = stmt->expr; | ||||||
| 	ParamListInfo paramLI; | 	volatile LocalTransactionId before_lxid; | ||||||
| 	LocalTransactionId before_lxid; |  | ||||||
| 	LocalTransactionId after_lxid; | 	LocalTransactionId after_lxid; | ||||||
| 	bool		pushed_active_snap = false; | 	volatile bool pushed_active_snap = false; | ||||||
| 	int			rc; | 	volatile int rc; | ||||||
|  |  | ||||||
| 	if (expr->plan == NULL) | 	/* PG_TRY to ensure we clear the plan link, if needed, on failure */ | ||||||
|  | 	PG_TRY(); | ||||||
| 	{ | 	{ | ||||||
| 		SPIPlanPtr	plan; | 		SPIPlanPtr	plan = expr->plan; | ||||||
|  | 		ParamListInfo paramLI; | ||||||
|  |  | ||||||
| 		/* | 		if (plan == NULL) | ||||||
| 		 * Don't save the plan if not in atomic context.  Otherwise, | 		{ | ||||||
| 		 * transaction ends would cause errors about plancache leaks.  XXX |  | ||||||
| 		 * This would be fixable with some plancache/resowner surgery |  | ||||||
| 		 * elsewhere, but for now we'll just work around this here. |  | ||||||
| 		 */ |  | ||||||
| 		exec_prepare_plan(estate, expr, 0, estate->atomic); |  | ||||||
|  |  | ||||||
| 		/* | 			/* | ||||||
| 		 * The procedure call could end transactions, which would upset the | 			 * Don't save the plan if not in atomic context.  Otherwise, | ||||||
| 		 * snapshot management in SPI_execute*, so don't let it do it. | 			 * transaction ends would cause errors about plancache leaks. | ||||||
| 		 * Instead, we set the snapshots ourselves below. | 			 * | ||||||
| 		 */ | 			 * XXX This would be fixable with some plancache/resowner surgery | ||||||
| 		plan = expr->plan; | 			 * elsewhere, but for now we'll just work around this here. | ||||||
| 		plan->no_snapshots = true; | 			 */ | ||||||
|  | 			exec_prepare_plan(estate, expr, 0, estate->atomic); | ||||||
|  |  | ||||||
|  | 			/* | ||||||
|  | 			 * The procedure call could end transactions, which would upset | ||||||
|  | 			 * the snapshot management in SPI_execute*, so don't let it do it. | ||||||
|  | 			 * Instead, we set the snapshots ourselves below. | ||||||
|  | 			 */ | ||||||
|  | 			plan = expr->plan; | ||||||
|  | 			plan->no_snapshots = true; | ||||||
|  |  | ||||||
|  | 			/* | ||||||
|  | 			 * Force target to be recalculated whenever the plan changes, in | ||||||
|  | 			 * case the procedure's argument list has changed. | ||||||
|  | 			 */ | ||||||
|  | 			stmt->target = NULL; | ||||||
|  | 		} | ||||||
|  |  | ||||||
| 		/* | 		/* | ||||||
| 		 * We construct a DTYPE_ROW datum representing the plpgsql variables | 		 * We construct a DTYPE_ROW datum representing the plpgsql variables | ||||||
| 		 * associated with the procedure's output arguments.  Then we can use | 		 * associated with the procedure's output arguments.  Then we can use | ||||||
| 		 * exec_move_row() to do the assignments.  (We do this each time the | 		 * exec_move_row() to do the assignments. | ||||||
| 		 * plan changes, in case the procedure's argument list has changed.) |  | ||||||
| 		 */ | 		 */ | ||||||
| 		if (stmt->is_call) | 		if (stmt->is_call && stmt->target == NULL) | ||||||
| 		{ | 		{ | ||||||
| 			Node	   *node; | 			Node	   *node; | ||||||
| 			FuncExpr   *funcexpr; | 			FuncExpr   *funcexpr; | ||||||
| @@ -2206,24 +2217,21 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt) | |||||||
|  |  | ||||||
| 			stmt->target = (PLpgSQL_variable *) row; | 			stmt->target = (PLpgSQL_variable *) row; | ||||||
| 		} | 		} | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	paramLI = setup_param_list(estate, expr); | 		paramLI = setup_param_list(estate, expr); | ||||||
|  |  | ||||||
| 	before_lxid = MyProc->lxid; | 		before_lxid = MyProc->lxid; | ||||||
|  |  | ||||||
| 	/* | 		/* | ||||||
| 	 * Set snapshot only for non-read-only procedures, similar to SPI | 		 * Set snapshot only for non-read-only procedures, similar to SPI | ||||||
| 	 * behavior. | 		 * behavior. | ||||||
| 	 */ | 		 */ | ||||||
| 	if (!estate->readonly_func) | 		if (!estate->readonly_func) | ||||||
| 	{ | 		{ | ||||||
| 		PushActiveSnapshot(GetTransactionSnapshot()); | 			PushActiveSnapshot(GetTransactionSnapshot()); | ||||||
| 		pushed_active_snap = true; | 			pushed_active_snap = true; | ||||||
| 	} | 		} | ||||||
|  |  | ||||||
| 	PG_TRY(); |  | ||||||
| 	{ |  | ||||||
| 		rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI, | 		rc = SPI_execute_plan_with_paramlist(expr->plan, paramLI, | ||||||
| 											 estate->readonly_func, 0); | 											 estate->readonly_func, 0); | ||||||
| 	} | 	} | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user