Horton

Thread

Commits

  1. Support CONSTANT/NOT NULL/initial value for plpgsql composite variables.

  1. CONSTANT/NOT NULL/initializer properties for plpgsql record variables

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-12-30T03:47:30Z

    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.
Attached is proof of the pudding.  This needs to be applied on top of
the patches in
https://postgr.es/m/23537.1514589378@sss.pgh.pa.us
and
https://postgr.es/m/11986.1514407114@sss.pgh.pa.us

It's pretty straightforward really.  Worth noting is that this also
fixes the null-domain-value issues I mentioned as being a loose end
in the first of the above-referenced messages.  Also, I created a
new plpgsql test file for these features, and moved the one relevant
existing test case into that file.

I was a bit disappointed to find that no documentation changes seem
needed, because the SGML docs fail to acknowledge that these cases
didn't work ...

			regards, tom lane

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-01-24T23:32:40Z

    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

			regards, tom lane

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

    Daniel Gustafsson <daniel@yesql.se> — 2018-01-25T11:40:28Z

    > On 25 Jan 2018, at 00:32, Tom Lane <tgl@sss.pgh.pa.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

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-01-25T15:34:33Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> 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.

Thanks for reviewing!

> 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”.

Hmm.  What would you imagine the hint saying?  It's certainly true that
a lot of people don't understand that a declared array length doesn't
mean anything in Postgres, but that's not specific to this context.

> 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).

The documentation currently says

	The CONSTANT option prevents the variable from being assigned to
	after initialization, so that its value will remain constant for
	the duration of the block.

I don't mind changing that if we can think of clearer wording, but to
me it seems pretty clear already.

			regards, tom lane

  5. CONSTANT/NOT NULL/initializer properties for plpgsql record variables

    David G. Johnston <david.g.johnston@gmail.com> — 2018-01-25T16:00:37Z

    On Thursday, January 25, 2018, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> The documentation currently says
>
>         The CONSTANT option prevents the variable from being assigned to
>         after initialization, so that its value will remain constant for
>         the duration of the block.
>

While we don't really have the concept of mutable types other languages
do.  Maybe just saying "from being assigned or mutated after
initialization" would suffice.

I don't see a desirable way to reinforce that the y in arr[y] is unenforced
documentation - in the code or docs - beyond what is already done.  That
distinction doesn't usually come up and for those where it does the docs
will probably be an after-the-fact confirmation of observed behavior.

David J.

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

    Daniel Gustafsson <daniel@yesql.se> — 2018-01-25T17:28:49Z

    > On 25 Jan 2018, at 16:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Daniel Gustafsson <daniel@yesql.se> writes:

>> 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”.
> 
> Hmm.  What would you imagine the hint saying?  It's certainly true that
> a lot of people don't understand that a declared array length doesn't
> mean anything in Postgres, but that's not specific to this context.

I was thinking about something along the lines of:

  errhint("In array declarations, CONSTANT refers to elements and not length."),

.. when datatype->typisarray is true, but again it might just be waffling that
doesn’t help anyone.

cheers ./daniel