mirror of
				https://github.com/postgres/postgres.git
				synced 2025-10-31 00:03:57 -04:00 
			
		
		
		
	Fix "cannot accept a set" error when only some arms of a CASE return a set.
In commit c1352052ef1d4eeb2eb1d822a207ddc2d106cb13, I implemented an optimization that assumed that a function's argument expressions would either always return a set (ie multiple rows), or always not. This is wrong however: we allow CASE expressions in which some arms return a set of some type and others just return a scalar of that type. There may be other examples as well. To fix, replace the run-time test of whether an argument returned a set with a static precheck (expression_returns_set). This adds a little bit of query startup overhead, but it seems barely measurable. Per bug #8228 from David Johnston. This has been broken since 8.0, so patch all supported branches.
This commit is contained in:
		
							parent
							
								
									daa7527afc
								
							
						
					
					
						commit
						080b7db72e
					
				| @ -1634,9 +1634,7 @@ tupledesc_match(TupleDesc dst_tupdesc, TupleDesc src_tupdesc) | |||||||
|  * init_fcache is presumed already run on the FuncExprState. |  * init_fcache is presumed already run on the FuncExprState. | ||||||
|  * |  * | ||||||
|  * This function handles the most general case, wherein the function or |  * This function handles the most general case, wherein the function or | ||||||
|  * one of its arguments might (or might not) return a set.	If we find |  * one of its arguments can return a set. | ||||||
|  * no sets involved, we will change the FuncExprState's function pointer |  | ||||||
|  * to use a simpler method on subsequent calls. |  | ||||||
|  */ |  */ | ||||||
| static Datum | static Datum | ||||||
| ExecMakeFunctionResult(FuncExprState *fcache, | ExecMakeFunctionResult(FuncExprState *fcache, | ||||||
| @ -1906,13 +1904,12 @@ restart: | |||||||
| 		/*
 | 		/*
 | ||||||
| 		 * Non-set case: much easier. | 		 * Non-set case: much easier. | ||||||
| 		 * | 		 * | ||||||
| 		 * We change the ExprState function pointer to use the simpler | 		 * In common cases, this code path is unreachable because we'd have | ||||||
| 		 * ExecMakeFunctionResultNoSets on subsequent calls.  This amounts to | 		 * selected ExecMakeFunctionResultNoSets instead.  However, it's | ||||||
| 		 * assuming that no argument can return a set if it didn't do so the | 		 * possible to get here if an argument sometimes produces set results | ||||||
| 		 * first time. | 		 * and sometimes scalar results.  For example, a CASE expression might | ||||||
|  | 		 * call a set-returning function in only some of its arms. | ||||||
| 		 */ | 		 */ | ||||||
| 		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets; |  | ||||||
| 
 |  | ||||||
| 		if (isDone) | 		if (isDone) | ||||||
| 			*isDone = ExprSingleResult; | 			*isDone = ExprSingleResult; | ||||||
| 
 | 
 | ||||||
| @ -2371,10 +2368,22 @@ ExecEvalFunc(FuncExprState *fcache, | |||||||
| 	init_fcache(func->funcid, func->inputcollid, fcache, | 	init_fcache(func->funcid, func->inputcollid, fcache, | ||||||
| 				econtext->ecxt_per_query_memory, true); | 				econtext->ecxt_per_query_memory, true); | ||||||
| 
 | 
 | ||||||
| 	/* Go directly to ExecMakeFunctionResult on subsequent uses */ | 	/*
 | ||||||
| 	fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult; | 	 * We need to invoke ExecMakeFunctionResult if either the function itself | ||||||
| 
 | 	 * or any of its input expressions can return a set.  Otherwise, invoke | ||||||
| 	return ExecMakeFunctionResult(fcache, econtext, isNull, isDone); | 	 * ExecMakeFunctionResultNoSets.  In either case, change the evalfunc | ||||||
|  | 	 * pointer to go directly there on subsequent uses. | ||||||
|  | 	 */ | ||||||
|  | 	if (fcache->func.fn_retset || expression_returns_set((Node *) func->args)) | ||||||
|  | 	{ | ||||||
|  | 		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult; | ||||||
|  | 		return ExecMakeFunctionResult(fcache, econtext, isNull, isDone); | ||||||
|  | 	} | ||||||
|  | 	else | ||||||
|  | 	{ | ||||||
|  | 		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets; | ||||||
|  | 		return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone); | ||||||
|  | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /* ----------------------------------------------------------------
 | /* ----------------------------------------------------------------
 | ||||||
| @ -2394,10 +2403,22 @@ ExecEvalOper(FuncExprState *fcache, | |||||||
| 	init_fcache(op->opfuncid, op->inputcollid, fcache, | 	init_fcache(op->opfuncid, op->inputcollid, fcache, | ||||||
| 				econtext->ecxt_per_query_memory, true); | 				econtext->ecxt_per_query_memory, true); | ||||||
| 
 | 
 | ||||||
| 	/* Go directly to ExecMakeFunctionResult on subsequent uses */ | 	/*
 | ||||||
| 	fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult; | 	 * We need to invoke ExecMakeFunctionResult if either the function itself | ||||||
| 
 | 	 * or any of its input expressions can return a set.  Otherwise, invoke | ||||||
| 	return ExecMakeFunctionResult(fcache, econtext, isNull, isDone); | 	 * ExecMakeFunctionResultNoSets.  In either case, change the evalfunc | ||||||
|  | 	 * pointer to go directly there on subsequent uses. | ||||||
|  | 	 */ | ||||||
|  | 	if (fcache->func.fn_retset || expression_returns_set((Node *) op->args)) | ||||||
|  | 	{ | ||||||
|  | 		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResult; | ||||||
|  | 		return ExecMakeFunctionResult(fcache, econtext, isNull, isDone); | ||||||
|  | 	} | ||||||
|  | 	else | ||||||
|  | 	{ | ||||||
|  | 		fcache->xprstate.evalfunc = (ExprStateEvalFunc) ExecMakeFunctionResultNoSets; | ||||||
|  | 		return ExecMakeFunctionResultNoSets(fcache, econtext, isNull, isDone); | ||||||
|  | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /* ----------------------------------------------------------------
 | /* ----------------------------------------------------------------
 | ||||||
|  | |||||||
| @ -1992,3 +1992,17 @@ select * from foobar();  -- fail | |||||||
| ERROR:  function return row and query-specified return row do not match | ERROR:  function return row and query-specified return row do not match | ||||||
| DETAIL:  Returned row contains 3 attributes, but query expects 2. | DETAIL:  Returned row contains 3 attributes, but query expects 2. | ||||||
| drop function foobar(); | drop function foobar(); | ||||||
|  | -- check behavior when a function's input sometimes returns a set (bug #8228) | ||||||
|  | SELECT *, | ||||||
|  |   lower(CASE WHEN id = 2 THEN (regexp_matches(str, '^0*([1-9]\d+)$'))[1] | ||||||
|  |         ELSE str | ||||||
|  |         END) | ||||||
|  | FROM | ||||||
|  |   (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str); | ||||||
|  |  id |       str        |      lower        | ||||||
|  | ----+------------------+------------------ | ||||||
|  |   1 |                  |  | ||||||
|  |   2 | 0000000049404    | 49404 | ||||||
|  |   3 | FROM 10000000876 | from 10000000876 | ||||||
|  | (3 rows) | ||||||
|  | 
 | ||||||
|  | |||||||
| @ -599,3 +599,12 @@ $$ select (1, 2.1, 3) $$ language sql; | |||||||
| select * from foobar();  -- fail | select * from foobar();  -- fail | ||||||
| 
 | 
 | ||||||
| drop function foobar(); | drop function foobar(); | ||||||
|  | 
 | ||||||
|  | -- check behavior when a function's input sometimes returns a set (bug #8228) | ||||||
|  | 
 | ||||||
|  | SELECT *, | ||||||
|  |   lower(CASE WHEN id = 2 THEN (regexp_matches(str, '^0*([1-9]\d+)$'))[1] | ||||||
|  |         ELSE str | ||||||
|  |         END) | ||||||
|  | FROM | ||||||
|  |   (VALUES (1,''), (2,'0000000049404'), (3,'FROM 10000000876')) v(id, str); | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user