Re: bug: virtual generated column can be partition key

On Tue, May 6, 2025 at 4:19 PM Yura Sokolov <y.sokolov@postgrespro.ru> wrote:

> > you may also see ComputePartitionAttrs.
>
> I saw it. And I gave links to comments in this function. This comments
> clearly state, there is no real reason to forbid GENERATED VIRTUAL columns
> (in opposite to STORED). They are forbidden just because author had no
> time/wish to finish their support.
>

Jian wrote:

>
> I posted a patch about virtual generated column as partition key in
> https://commitfest.postgresql.org/patch/5720/
>
> but in this context, we still need error out for virtual generated column.
> maybe we can change the error code, like the below,
> but i feel it's not necessary.


RIght. There is no reason to not support partition keys containing
virtual columns. It's just a limitation in backbranches. It will be
removed by Jian's proposal.

However, in the backbranches we do not allow a virtual generated
column to be part of partition key when it appears directly in the
partition key as a column or in an expression. By that same reasoning,
a whole row reference to a relation having a virtual column should not
be allowed since whole row reference is a row expression containing
all the columns of that relation. That's what this thread and the
patch is about.

>
> ComputePartitionAttrs
>                 if (TupleDescAttr(RelationGetDescr(rel), attno -
> 1)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
>                     ereport(ERROR,
>                             errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>                             errmsg("cannot use generated column in
> partition key"),
>                             parser_errposition(pstate, pelem->location));
>
>                 if (TupleDescAttr(RelationGetDescr(rel), attno -
> 1)->attgenerated)
>                     ereport(ERROR,
>                             (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
>                              errmsg("cannot use generated column in
> partition key"),
>                              errdetail("Column \"%s\" is a generated column.",
>
> get_attname(RelationGetRelid(rel), attno, false)),
>                              parser_errposition(pstate, pelem->location)));
>

Hmm, that may be considered backward compatibility breaking change
since it's changing an error code that a user may be relying on. I
would avoid such a change in backbranches.

-- 
Best Wishes,
Ashutosh Bapat