From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrey Borodin <x4mmm@yandex-team.ru>
cc: Michel Pelletier <pelletier.michel@gmail.com>,
        Pavel Stehule <pavel.stehule@gmail.com>,
        pgsql-hackers@lists.postgresql.org
Subject: Re: Using Expanded Objects other than Arrays from plpgsql
In-reply-to: <0AC229FA-A3F1-43FD-B0DC-A46A73FEAFF7@yandex-team.ru>
References: <CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com> <1342498.1729444411@sss.pgh.pa.us> <CACxu=vLXvpzN4X3k+9jsMt6ujuOvFVUSkA80t_cROSsF4y2jQQ@mail.gmail.com> <1445998.1729482404@sss.pgh.pa.us> <CACxu=vKEF8Qa-OaADFxf0uMg-xw6gH_CNCWd2s+xaqh-gY4=xg@mail.gmail.com> <2062830.1729625620@sss.pgh.pa.us> <2265411.1729699470@sss.pgh.pa.us> <CACxu=v++HNmss59yGUDkRny7g=M8tZ2YXF07AUXqKVGqcSfxGQ@mail.gmail.com> <2354718.1729737539@sss.pgh.pa.us> <2581216.1729794746@sss.pgh.pa.us> <CACxu=v+dn37zr8gx5xNP-EZY3OLtGLTHrbx_ZkCQc40HpyMLKA@mail.gmail.com> <1948345.1730500073@sss.pgh.pa.us> <CACxu=vL7i_U_iSNpREe8eCAMEyKHuPpk9THRpBhk+ar0U1EdOw@mail.gmail.com> <CACxu=vKLc6f5N8_DR58LKkE1eohWSxTvThTeGsLm7p7QH1aFBA@mail.gmail.com> <CACxu=vJf2S=ysun_h=zmYNu6oUM47+egbpX5mMC0X9BJK=EQwQ@mail.gmail.com> <CACxu=vK+S6BXN8ZYyBvqQBWrcwHXqtue1-ZuKO3+XtHGBYcDUQ@mail.gmail.com> <CAFj8pRCd6xcH-AYEyHFdGdU89O9JjZ-v-pyQnOwd9zNJkCEdhQ@mail.gmail.com> <3797606.1732045516@sss.pgh.pa.us> <647219!
 ! .1736019347@sss.pgh.pa.us> <CACxu=vJ1UeXPjPOzfT2E=tFpSCxPxcB7hgfqHYmefJWh3ZXPew@mail.gmail.com> <1417389.1736964543@sss.pgh.pa.us> <CACxu=vKModwqSpEjpn-ur1uMAEYkcd-i8CvNsH-z9=dk7F7YQw@mail.gmail.com> <3363452.1737483125@sss.pgh.pa.us> <0AC229FA-A3F1-43FD-B0DC-A46A73FEAFF7@yandex-team.ru>
Comments: In-reply-to Andrey Borodin <x4mmm@yandex-team.ru>
	message dated "Sun, 26 Jan 2025 15:07:22 +0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <931397.1737905825.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 26 Jan 2025 10:37:05 -0500
Message-ID: <931398.1737905825@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/931398.1737905825%40sss.pgh.pa.us>
Precedence: bulk

Andrey Borodin <x4mmm@yandex-team.ru> writes:
>> On 21 Jan 2025, at 23:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> somebody will review this

> I'm trying to dig into the patch set. My knowledge of the module is shal=
low and I hope to improve it by reading more patches in this area.

Thanks for looking!

> And the coverage of newly invented mark_stmt() 42.37%. Some of branches =
are easy noops, but some are not.

Yeah.  I'm not too concerned about that because it's pretty much a
copy-and-paste of the adjacent code.  Maybe we should think about
some way of refactoring pl_funcs.c to reduce duplication, but I
don't have any great ideas about how.

> expr_is_assignment_source() is named like if it should return nool, but =
it's void.

I've been less than satisfied with that name too.  I intended it
as a statement of fact, "this expression has been found to be =

the source of an assignment".  But it does seem confusing.
Maybe we should recast it as an action.  What do you think of
"mark_expr_as_assignment_source"?

> I could not grasp from reading the code one generic question about new o=
ptimization rule. What cost does checking for possible in-place update inc=
urs to code cannot have this optimization? Is it O(numer_of_arguments) of =
for every assignment execution?

No, the extra effort is incurred at most once per assignment statement
per session.  (Unless the plpgsql function's cache entry gets
invalidated, in which case we'd rebuild all of the function's data
structures and have to redo this work too.)  We set up the evaluation
function "paramfunc" as plpgsql_param_eval_var_check if we think we
might be able to apply this optimization, or plpgsql_param_eval_var_ro
if we don't think so but the variable is of varlena type.  At runtime,
if the variable's current value is not actually expanded, then
plpgsql_param_eval_var_check falls through doing essentially the same
work as plpgsql_param_eval_var_ro, so there should be no added cost.
The first time we observe that the value *is* expanded, we incur the
cost to detect whether an optimization is really possible, and then
we change the "paramfunc" pointer to be the appropriate function
so as to apply the optimization or not without rechecking.  So
generally speaking, if we're considering a variable of a type that
doesn't support expansion, there should be zero extra per-execution
cost.  There is some extra cost at function compilation time to
determine which expressions are assignment sources (but we were doing
that already) and to discover whether those assignments are to
nonlocal variables (which is new work, but only needs to be done in
functions with exception blocks).

			regards, tom lane