From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
Cc: | Michel Pelletier <pelletier(dot)michel(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Using Expanded Objects other than Arrays from plpgsql |
Date: | 2025-01-26 15:37:05 |
Message-ID: | 931398.1737905825@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-general pgsql-hackers |
Andrey Borodin <x4mmm(at)yandex-team(dot)ru> writes:
>> On 21 Jan 2025, at 23:12, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> somebody will review this
> I'm trying to dig into the patch set. My knowledge of the module is shallow 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 optimization rule. What cost does checking for possible in-place update incurs 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
From | Date | Subject | |
---|---|---|---|
Next Message | Andrey Borodin | 2025-01-26 17:04:15 | Re: Using Expanded Objects other than Arrays from plpgsql |
Previous Message | Дмитрий | 2025-01-26 11:29:16 | Re[2]: FATAL: could not send data to WAL stream: lost synchronization with server: got message type "0", length 892351284 |
From | Date | Subject | |
---|---|---|---|
Next Message | Mahendra Singh Thalor | 2025-01-26 15:48:16 | Re: Non-text mode for pg_dumpall |
Previous Message | Joe Conway | 2025-01-26 15:29:34 | Re: Convert sepgsql tests to TAP |