Re: CONSTANT/NOT NULL/initializer properties for plpgsql record variables

> On 25 Jan 2018, at 00:32, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> I wrote:
>> I said a couple of times in recent threads that it wouldn't be too hard
>> to implement $SUBJECT given the other patches I've been working on.
>
> Here's a version rebased up to HEAD, with a trivial merge conflict fixed.
>
> This now needs to be applied over the patches in
> https://postgr.es/m/833.1516834367@sss.pgh.pa.us
> and
> https://postgr.es/m/8376.1516835784@sss.pgh.pa.us

I’ve reviewed this patch (disclaimer: I did not review the patches listed above
which it is based on) and the functionality introduced. The code is straight-
forward, there are ample tests and I can’t make it break however many weird
combinations thrown at it. Regarding the functionality it’s clearly a +1 on
getting this in.

One tiny thing: while not introduced in this patch, I wonder if it would be
worth adding an errhint in the following hunk when applied to arrays, to
clarify what CONSTANT in an array declaration mean. I have seen confusion
around what y means in ‘x int[y]’, and I can see ‘x CONSTANT int[y]’ being
misinterpreted as “length fixed to y”.

- errmsg("\"%s\" is declared CONSTANT",
- ((PLpgSQL_var *) datum)->refname),
+ errmsg("variable \"%s\" is declared CONSTANT",
+ ((PLpgSQL_variable *) datum)->refname),

That might not be a common enough misunderstanding to warrant it, but it was
the only thing that stood out to me (and perhaps it would be better in the docs
if done at all).

Setting this ready for committer, with the caveat that the underlying patches
must go in of course.

cheers ./daniel