public inbox for [email protected]
help / color / mirror / Atom feedFrom: Pavel Borisov <[email protected]>
To: Tom Lane <[email protected]>
Cc: Michel Pelletier <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Pavel Stehule <[email protected]>
Cc: [email protected]
Subject: Re: Using Expanded Objects other than Arrays from plpgsql
Date: Tue, 4 Feb 2025 15:54:52 +0400
Message-ID: <CALT9ZEGn2rV2WQp+Zh6UqgxJ-FdpgLxGkNL7kOfd3aoFQ=5h4w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com>
<[email protected]>
<CACxu=vLXvpzN4X3k+9jsMt6ujuOvFVUSkA80t_cROSsF4y2jQQ@mail.gmail.com>
<[email protected]>
<CACxu=vKEF8Qa-OaADFxf0uMg-xw6gH_CNCWd2s+xaqh-gY4=xg@mail.gmail.com>
<[email protected]>
<[email protected]>
<CACxu=v++HNmss59yGUDkRny7g=M8tZ2YXF07AUXqKVGqcSfxGQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<CACxu=v+dn37zr8gx5xNP-EZY3OLtGLTHrbx_ZkCQc40HpyMLKA@mail.gmail.com>
<[email protected]>
<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>
<[email protected]>
<CACxu=vKModwqSpEjpn-ur1uMAEYkcd-i8CvNsH-z9=dk7F7YQw@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<CALT9ZEGy5+GNQUtR80BPL7k-Qi47sqwPLnN_dOaQUW5UbxcTGA@mail.gmail.com>
<[email protected]>
<[email protected]>
<CACxu=vJ250gdGF0HPzBEjCX+dDWVvum3P4Vuxg+B_VJ7x69h8A@mail.gmail.com>
<CALT9ZEE1+e2pn5F9qTrc0n5DqD++EzzoyoKG8aS2xA8_yXUxFA@mail.gmail.com>
<[email protected]>
Hi, Tom!
On Mon, 3 Feb 2025 at 21:53, Tom Lane <[email protected]> wrote:
>
> Pavel Borisov <[email protected]> writes:
> > Minor notes on the patches:
>
> > If dump_* functions could use the newly added walker, the code would
> > look better. I suppose the main complication is that dump_* functions
> > contain a lot of per-statement prints/formatting. So maybe a way to
> > implement this is to put these statements into the existing tree
> > walker i.e. plpgsql_statement_tree_walker_impl() and add an argument
> > bool dump_debug into it. So in effect called with dump_debug=false
> > plpgsql_statement_tree_walker_impl() would walk silent, and with
> > dump_debug=false it would walk and print what is supposed to be
> > printed currently in dump_* functions. Maybe there are other problems
> > besides this?
>
> I'm not thrilled with that idea, mainly because it would add overhead
> to the performance-relevant cases (mark and free) to benefit a rarely
> used debugging feature. I'm content to leave the debug code out of
> this for now --- it seems to me that it's serving a different master
> and doesn't have to be unified with the other routines.
Makes sense.
> > For exec_check_rw_parameter():
>
> > I think renaming expr->expr_simple_expr to sexpr saves few bytes but
> > doesn't makes anything simpler, so if possible I'd prefer using just
> > expr->expr_simple_expr with necessary casts. Furtermore in this
> > function mostly we use cast results fexpr, opexpr and sbsref of
> > expr->expr_simple_expr that already has separate names.
>
> Hmm, I thought it looked cleaner like this, but I agree beauty
> is in the eye of the beholder. Anybody else have a preference?
>
> > Transferring target param as int paramid looks completely ok. But we
> > have unconditional checking Assert(paramid == expr->target_param + 1),
> > so it looks as a redundant split as of now. Do we plan a true split
> > and removal of this assert in the future?
>
> We've already fetched the target variable using the paramid (cf
> plpgsql_param_eval_var_check), so I think checking that the
> expression does match it seems like a useful sanity check.
> Agreed, it shouldn't ever not match, but that's why that's just
> an Assert.
There are no problems with these.
Regards,
Pavel Borisov
view thread (34+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Using Expanded Objects other than Arrays from plpgsql
In-Reply-To: <CALT9ZEGn2rV2WQp+Zh6UqgxJ-FdpgLxGkNL7kOfd3aoFQ=5h4w@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox