Re: Misleading errors with column references in default expressions and partition bounds - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Misleading errors with column references in default expressions and partition bounds
Date
Msg-id 17662.1553609015@sss.pgh.pa.us
Whole thread Raw
In response to Misleading errors with column references in default expressions andpartition bounds  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
Michael Paquier <michael@paquier.xyz> writes:
> One idea which came from Amit, and it seems to me that it is a good
> idea, would be to have more context-related error messages directly in
> transformColumnRef(), so as we can discard at an early stage column
> references which are part of contexts where there is no meaning to
> have them.

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

> While this takes care of the RTE issues, this has a downside though.
> Take for example this case using an expression with an aggregate and
> a column reference: 
> =# CREATE TABLE part_bogus_expr_fail PARTITION OF list_parted
>    FOR VALUES IN (sum(a));
> -ERROR:  aggregate functions are not allowed in partition bound
> +ERROR:  cannot use column reference in partition bound expression

I don't see that as an issue.

> The docs of CREATE TABLE also look incorrect to me when it comes to
> default expressions.  It says the following: "other columns in the
> current table are not allowed".  However *all* columns are not
> authorized, including the column which uses the expression.

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.

            regards, tom lane


pgsql-hackers by date:

Previous
From: Dean Rasheed
Date:
Subject: Re: [HACKERS] PATCH: multivariate histograms and MCV lists
Next
From: Alexander Korotkov
Date:
Subject: Re: jsonpath