Re: Remove redundant code in pl_exec.c

2022-09-09 Thread Tom Lane
Japin Li  writes:
> On Fri, 09 Sep 2022 at 23:34, Tom Lane  wrote:
>> I don't like this particularly --- it puts way too much premium on
>> the happenstance that the MakeExpandedObjectReadOnly call is the
>> very last step in the callback function.  If that needed to change,
>> we'd have a mess.

> Sorry, I don't get your mind.  Could you explain it more?  Thanks in advance!

This refactoring cannot support the situation where there is more
code to execute after MakeExpandedObjectReadOnly.

regards, tom lane




Re: Remove redundant code in pl_exec.c

2022-09-09 Thread Japin Li


On Fri, 09 Sep 2022 at 23:34, Tom Lane  wrote:
> Japin Li  writes:
>> IMO, we can invoke plpgsql_param_eval_generic in 
>> plpgsql_param_eval_generic_ro
>> to avoid the redundant.
>
> I don't like this particularly --- it puts way too much premium on
> the happenstance that the MakeExpandedObjectReadOnly call is the
> very last step in the callback function.  If that needed to change,
> we'd have a mess.
>

Sorry, I don't get your mind.  Could you explain it more?  Thanks in advance!

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.




Re: Remove redundant code in pl_exec.c

2022-09-09 Thread Tom Lane
Japin Li  writes:
> I found there are some redundant code in pl_exec.c,
> plpgsql_param_eval_generic_ro is same as plpgsql_param_eval_generic
> except it invokes MakeExpandedObjectReadOnly.

Which is exactly why it's NOT redundant.

> IMO, we can invoke plpgsql_param_eval_generic in plpgsql_param_eval_generic_ro
> to avoid the redundant.

I don't like this particularly --- it puts way too much premium on
the happenstance that the MakeExpandedObjectReadOnly call is the
very last step in the callback function.  If that needed to change,
we'd have a mess.

regards, tom lane




Remove redundant code in pl_exec.c

2022-09-09 Thread Japin Li

Hi, hackers

I found there are some redundant code in pl_exec.c,
plpgsql_param_eval_generic_ro is same as plpgsql_param_eval_generic
except it invokes MakeExpandedObjectReadOnly.

IMO, we can invoke plpgsql_param_eval_generic in plpgsql_param_eval_generic_ro
to avoid the redundant.

Is there something I missed?  Any thoughts?

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 7bd2a9fff1..543419d3da 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -6673,34 +6673,7 @@ static void
 plpgsql_param_eval_generic_ro(ExprState *state, ExprEvalStep *op,
 			  ExprContext *econtext)
 {
-	ParamListInfo params;
-	PLpgSQL_execstate *estate;
-	int			dno = op->d.cparam.paramid - 1;
-	PLpgSQL_datum *datum;
-	Oid			datumtype;
-	int32		datumtypmod;
-
-	/* fetch back the hook data */
-	params = econtext->ecxt_param_list_info;
-	estate = (PLpgSQL_execstate *) params->paramFetchArg;
-	Assert(dno >= 0 && dno < estate->ndatums);
-
-	/* now we can access the target datum */
-	datum = estate->datums[dno];
-
-	/* fetch datum's value */
-	exec_eval_datum(estate, datum,
-	, ,
-	op->resvalue, op->resnull);
-
-	/* safety check -- needed for, eg, record fields */
-	if (unlikely(datumtype != op->d.cparam.paramtype))
-		ereport(ERROR,
-(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("type of parameter %d (%s) does not match that when preparing the plan (%s)",
-		op->d.cparam.paramid,
-		format_type_be(datumtype),
-		format_type_be(op->d.cparam.paramtype;
+	plpgsql_param_eval_generic(state, op, econtext);
 
 	/* force the value to read-only */
 	*op->resvalue = MakeExpandedObjectReadOnly(*op->resvalue,