Re: plpgsql: penalty due to double evaluation of parameters

Hi,

Within exec_prepare_plan there are calls to exec_eval_datum to identify the
argtypes of the involved parameters. However exec_eval_datum actually fills
up value, isnull entries in these cases causing unnecessary additional calls
when all we need is the datum type. Such unnecessary evaluation of values
might prove to be very costly later since this quirk of exec_eval_datum
usage is not so visible. Worse still it could cause bugs if some evaluations
have side-effects across multiple evals. It might make sense to introduce a
new function exec_eval_datum_type to address this or exec_eval_datum could
itself be modified for cases where we just need the datum type. Should I
cook up a patch for this? I am inclined towards introducing a new function
(but that means that any new datum related changes need to be carried out in
2 functions instead of one currently).

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

Nikhils wrote:
> Within exec_prepare_plan there are calls to exec_eval_datum to identify the
> argtypes of the involved parameters. However exec_eval_datum actually fills
> up value, isnull entries in these cases causing unnecessary additional calls
> when all we need is the datum type. Such unnecessary evaluation of values
> might prove to be very costly later since this quirk of exec_eval_datum
> usage is not so visible. Worse still it could cause bugs if some evaluations
> have side-effects across multiple evals.

I don't buy the performance argument unless I see some test results
demonstrating it; exec_prepare_plan is only called on the first
invocation of a statement. What kind of side-effects could
exec_eval_datum call have?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Hi,

I don't buy the performance argument unless I see some test results
> demonstrating it; exec_prepare_plan is only called on the first invocation
> of a statement. What kind of side-effects could exec_eval_datum call have?
>

Note that I have avoided using the "performance" word for this very reason.
But consider for example when the datum type is PLPGSQL_DTYPE_REC. I dont
think its justified to have the overhead of heap_copytuple_with_tuple, when
all we need is just the typeid! Similar arguments apply for other datums
like PLPGSQL_DTYPE_ROW, PLPGSQL_DTYPE_TRIGARG e.g.

Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com

"Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> writes:
> I don't buy the performance argument unless I see some test results
> demonstrating it; exec_prepare_plan is only called on the first
> invocation of a statement. What kind of side-effects could
> exec_eval_datum call have?

In principle, if you subscript TG_ARGV[] with an expression that has
visible side-effects, you could have unexpected behavior here. I think
though that the right fix is to get rid of the special treatment of
TG_ARGV[] and make it a ordinary variable with type text[], instead
of being its very own class of datum. The current implementation has
a lot of other misbehaviors for TG_ARGV[], like not being able to apply
array operations to it.

I agree that worrying about the performance is pointless, considering
the cost of the SPI_prepare_cursor() that we're about to do in the one
place where there could be a use for a get-the-type-only call.

regards, tom lane