Uniforms the errors msgs at tuplestore paths

Started by Ranier Vilelaalmost 4 years ago7 messages
#1Ranier Vilela
ranier.vf@gmail.com
1 attachment(s)

Hi,

Like how the commit
https://github.com/postgres/postgres/commit/07daca53bfcad59618a9c6fad304e380cc9d2bc1
The are some paths that were missed:

-At jsonfuncs.c
The errors mgs do not report about the materialize mode.
-At plperl.c
The function plperl_return_next_internal does not check rsi appropriately.
The error about materialize mode required, is missed.
-At pl_exec.c
The error mgs do not report about the materialize mode
-At pltcl.c:
The function pltcl_init_tuple_store does not check rsi appropriately.

Patch attached.

regards,
Ranier Vilela

Attachments:

v1-0001-uniform-error-msgs-tuplestore.patchapplication/octet-stream; name=v1-0001-uniform-error-msgs-tuplestore.patchDownload
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 0273f883d4..c4fd8a9162 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -1927,13 +1927,16 @@ each_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname, bool as_text)
 
 	rsi = (ReturnSetInfo *) fcinfo->resultinfo;
 
-	if (!rsi || !IsA(rsi, ReturnSetInfo) ||
-		(rsi->allowedModes & SFRM_Materialize) == 0 ||
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsi->allowedModes & SFRM_Materialize) ||
 		rsi->expectedDesc == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("set-valued function called in context that "
-						"cannot accept a set")));
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
 	rsi->returnMode = SFRM_Materialize;
 
@@ -2039,13 +2042,16 @@ each_worker(FunctionCallInfo fcinfo, bool as_text)
 
 	rsi = (ReturnSetInfo *) fcinfo->resultinfo;
 
-	if (!rsi || !IsA(rsi, ReturnSetInfo) ||
-		(rsi->allowedModes & SFRM_Materialize) == 0 ||
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsi->allowedModes & SFRM_Materialize) ||
 		rsi->expectedDesc == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("set-valued function called in context that "
-						"cannot accept a set")));
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
 	rsi->returnMode = SFRM_Materialize;
 
@@ -2227,13 +2233,16 @@ elements_worker_jsonb(FunctionCallInfo fcinfo, const char *funcname,
 
 	rsi = (ReturnSetInfo *) fcinfo->resultinfo;
 
-	if (!rsi || !IsA(rsi, ReturnSetInfo) ||
-		(rsi->allowedModes & SFRM_Materialize) == 0 ||
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsi->allowedModes & SFRM_Materialize) ||
 		rsi->expectedDesc == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("set-valued function called in context that "
-						"cannot accept a set")));
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
 	rsi->returnMode = SFRM_Materialize;
 
@@ -2336,13 +2345,16 @@ elements_worker(FunctionCallInfo fcinfo, const char *funcname, bool as_text)
 
 	rsi = (ReturnSetInfo *) fcinfo->resultinfo;
 
-	if (!rsi || !IsA(rsi, ReturnSetInfo) ||
-		(rsi->allowedModes & SFRM_Materialize) == 0 ||
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsi->allowedModes & SFRM_Materialize) ||
 		rsi->expectedDesc == NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("set-valued function called in context that "
-						"cannot accept a set")));
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
 	rsi->returnMode = SFRM_Materialize;
 
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 3f785b1e8d..0810ec9ccc 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -3257,6 +3257,11 @@ plperl_return_next_internal(SV *sv)
 				(errcode(ERRCODE_SYNTAX_ERROR),
 				 errmsg("cannot use return_next in a non-SETOF function")));
 
+	if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+
 	if (!current_call_data->ret_tdesc)
 	{
 		TupleDesc	tupdesc;
@@ -3286,9 +3291,15 @@ plperl_return_next_internal(SV *sv)
 		}
 		else
 		{
+			if (!(rsi->allowedModes & SFRM_Materialize) ||
+				rsi->expectedDesc == NULL)
+				ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("materialize mode required, but it is not allowed in this context")));
+
 			tupdesc = rsi->expectedDesc;
 			/* Protect assumption below that we return exactly one column */
-			if (tupdesc == NULL || tupdesc->natts != 1)
+			if (tupdesc->natts != 1)
 				elog(ERROR, "expected single-column result descriptor for non-composite SETOF result");
 		}
 
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 70c4a75295..e346614126 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3645,12 +3645,15 @@ exec_init_tuple_store(PLpgSQL_execstate *estate)
 	/*
 	 * Check caller can handle a set result in the way we want
 	 */
-	if (!rsi || !IsA(rsi, ReturnSetInfo) ||
-		(rsi->allowedModes & SFRM_Materialize) == 0 ||
-		rsi->expectedDesc == NULL)
+	if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsi->allowedModes & SFRM_Materialize) ||
+		rsi->expectedDesc == NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("materialize mode required, but it is not allowed in this context")));
 
 	/*
 	 * Switch to the right memory context and resource owner for storing the
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 7c045f4560..24c1085de5 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -3263,7 +3263,16 @@ pltcl_init_tuple_store(pltcl_call_state *call_state)
 	Assert(!call_state->attinmeta);
 
 	/* We expect caller to provide an appropriate result tupdesc */
-	Assert(rsi->expectedDesc);
+	if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("set-valued function called in context that cannot accept a set")));
+	if (!(rsi->allowedModes & SFRM_Materialize) ||
+		rsi->expectedDesc == NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("materialize mode required, but it is not allowed in this context")));
+
 	call_state->ret_tupdesc = rsi->expectedDesc;
 
 	/*
#2Justin Pryzby
pryzby@telsasoft.com
In reply to: Ranier Vilela (#1)
Re: Uniforms the errors msgs at tuplestore paths

On Sun, Feb 20, 2022 at 11:12:42AM -0300, Ranier Vilela wrote:

Like how the commit
https://github.com/postgres/postgres/commit/07daca53bfcad59618a9c6fad304e380cc9d2bc1
The are some paths that were missed:

I think these are all unified by the existing tuplestore patch.
https://commitfest.postgresql.org/37/3420/

Show quoted text

-At jsonfuncs.c
The errors mgs do not report about the materialize mode.
-At plperl.c
The function plperl_return_next_internal does not check rsi appropriately.
The error about materialize mode required, is missed.
-At pl_exec.c
The error mgs do not report about the materialize mode
-At pltcl.c:
The function pltcl_init_tuple_store does not check rsi appropriately.

#3Ranier Vilela
ranier.vf@gmail.com
In reply to: Justin Pryzby (#2)
Re: Uniforms the errors msgs at tuplestore paths

Em dom., 20 de fev. de 2022 às 11:30, Justin Pryzby <pryzby@telsasoft.com>
escreveu:

On Sun, Feb 20, 2022 at 11:12:42AM -0300, Ranier Vilela wrote:

Like how the commit

https://github.com/postgres/postgres/commit/07daca53bfcad59618a9c6fad304e380cc9d2bc1

The are some paths that were missed:

I think these are all unified by the existing tuplestore patch.
https://commitfest.postgresql.org/37/3420/

I can't see:
plperl.c
pl_exec.c
pttcl.c

Only jsonfuncs.c, but the error about "materialized mode" is not reported.

regards,
Ranier Vilela

#4Michael Paquier
michael@paquier.xyz
In reply to: Ranier Vilela (#3)
Re: Uniforms the errors msgs at tuplestore paths

On Sun, Feb 20, 2022 at 11:37:33AM -0300, Ranier Vilela wrote:

I can't see:
plperl.c
pl_exec.c
pttcl.c

Only jsonfuncs.c, but the error about "materialized mode" is not reported.

Melanie has done a nice analysis of all the code paths doing
materialization checks for her patch with SRF functions. Though there
are parts that can be simplified to reduce the differences in check
patterns before doing the overall refactoring, I think that we'd
better keep any discussion related to this topic on the other thread
rather than splitting the effort across more threads.
--
Michael

#5Ranier Vilela
ranier.vf@gmail.com
In reply to: Michael Paquier (#4)
Re: Uniforms the errors msgs at tuplestore paths

Em dom., 20 de fev. de 2022 às 22:08, Michael Paquier <michael@paquier.xyz>
escreveu:

On Sun, Feb 20, 2022 at 11:37:33AM -0300, Ranier Vilela wrote:

I can't see:
plperl.c
pl_exec.c
pttcl.c

Only jsonfuncs.c, but the error about "materialized mode" is not

reported.

Melanie has done a nice analysis of all the code paths doing
materialization checks for her patch with SRF functions. Though there
are parts that can be simplified to reduce the differences in check
patterns before doing the overall refactoring, I think that we'd
better keep any discussion related to this topic on the other thread
rather than splitting the effort across more threads.

Fine, I agree.

regards,
Ranier Vilela

#6Ranier Vilela
ranier.vf@gmail.com
In reply to: Ranier Vilela (#5)
Re: Uniforms the errors msgs at tuplestore paths

Em dom., 20 de fev. de 2022 às 22:45, Ranier Vilela <ranier.vf@gmail.com>
escreveu:

Em dom., 20 de fev. de 2022 às 22:08, Michael Paquier <michael@paquier.xyz>
escreveu:

On Sun, Feb 20, 2022 at 11:37:33AM -0300, Ranier Vilela wrote:

I can't see:
plperl.c
pl_exec.c
pttcl.c

Only jsonfuncs.c, but the error about "materialized mode" is not

reported.

Melanie has done a nice analysis of all the code paths doing
materialization checks for her patch with SRF functions. Though there
are parts that can be simplified to reduce the differences in check
patterns before doing the overall refactoring, I think that we'd
better keep any discussion related to this topic on the other thread
rather than splitting the effort across more threads.

Fine, I agree.

Thanks for the commit Michael.

regards,
Ranier Vilela

Show quoted text
#7Michael Paquier
michael@paquier.xyz
In reply to: Ranier Vilela (#6)
Re: Uniforms the errors msgs at tuplestore paths

On Thu, Feb 24, 2022 at 08:30:02AM -0300, Ranier Vilela wrote:

Thanks for the commit Michael.

No problem. For the archives, this is e77216f.
--
Michael