Re: Misleading errors with column references in default expressions and partition bounds

On Tue, Mar 26, 2019 at 10:03:35AM -0400, Tom Lane wrote:
> +1 for the general idea, but I find the switch a bit overly verbose.
> Do we really need to force every new EXPR_KIND to visit this spot,
> when so few of them have a need to do anything?  I'd be a bit inclined
> to simplify it to
> 
> 	switch (pstate->p_expr_kind)
> 	{
> 		case EXPR_KIND_COLUMN_DEFAULT:
> 			ereport(...);
> 			break;
> 		case EXPR_KIND_PARTITION_BOUND:
> 			ereport(...);
> 			break;
> 		default:
> 			break;
> 	}
> 
> That's just a nitpick though.

ParseExprKind is an enum, so listing all the options without the
default has the advantage to generate a warning if somebody adds a
value.  This way anybody changing this code will need to think about
it.

> I don't see that as an issue.

Thanks!

> I think the idea is that trying to reference another column is something
> that people might well try to do, whereas referencing the DEFAULT's
> own column is obviously silly.  In particular the use of "cross-reference"
> implies that another column is what is being referenced.  If we dumb it
> down to just "references to columns in the current table", then it's
> consistent, but it's also completely redundant with the main part of the
> sentence.  It doesn't help that somebody decided to jam the independent
> issue of subqueries into the same sentence.  In short, maybe it'd be
> better like this:
> 
>        ... The value
>        is any variable-free expression (in particular, cross-references
>        to other columns in the current table are not allowed).  Subqueries
>        are not allowed either.

Okay, I think I see your point here.  That sounds sensible.
--
Michael