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

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: Misleading errors with column references in default expressions and partition bounds
Дата
Msg-id 17662.1553609015@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Misleading errors with column references in default expressions andpartition bounds  (Michael Paquier <michael@paquier.xyz>)
Список 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 по дате отправления:

Предыдущее
От: Dean Rasheed
Дата:
Сообщение: Re: [HACKERS] PATCH: multivariate histograms and MCV lists
Следующее
От: Alexander Korotkov
Дата:
Сообщение: Re: jsonpath