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