Thread: Remove redundant code in pl_exec.c
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,
- &datumtype, &datumtypmod,
- 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,
Japin Li <japinli@hotmail.com> 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
On Fri, 09 Sep 2022 at 23:34, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Japin Li <japinli@hotmail.com> 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.
Japin Li <japinli@hotmail.com> writes:
> On Fri, 09 Sep 2022 at 23:34, Tom Lane <tgl@sss.pgh.pa.us> 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