Re: bug: virtual generated column can be partition key

On Tue, May 6, 2025 at 6:49 PM Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> wrote:
>
> 06.05.2025 13:31, jian he пишет:
> > On Tue, May 6, 2025 at 5:57 PM Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru> wrote:
> >>
> >> 21.04.2025 05:30, jian he пишет:
> >>> hi.
> >>> While trying to make the virtual generated column be part of the partition key,
> >>> I found this bug.
> >>> it also influences the stored generated column, i added a test
> >>> on generated_stored.sql.
> >>>
> >>> CREATE TABLE gtest_part_key (
> >>> f1 date NOT NULL, f2 bigint,
> >>> f3 bigint GENERATED ALWAYS AS (f2 * 2) VIRTUAL)
> >>> PARTITION BY RANGE (f3);
> >>>
> >>> ERROR: cannot use generated column in partition key
> >>> LINE 4: PARTITION BY RANGE (f3);
> >>> ^
> >>> DETAIL: Column "f3" is a generated column.
> >>>
> >>> the following is essentially the same as above, it should also fail.
> >>>
> >>> CREATE TABLE gtest_part_key (
> >>> f1 date NOT NULL, f2 bigint,
> >>> f3 bigint GENERATED ALWAYS AS (f2 * 2) VIRTUAL)
> >>> PARTITION BY RANGE ((f3));
> >>
> >> I don't understand why latter should fail?
> >>
> >> Documentation says [1]:
> >>
> >>> PostgreSQL allows you to declare that a table is divided into partitions.
> >>> The table that is divided is referred to as a partitioned table.
> >>> The declaration includes the partitioning method as described above,
> >>> plus a list of columns or expressions to be used as the partition key.
> >>
> >> Note: "list of columns or EXPRESSIONS"!
> >>
> >> In first case you pass list of columns (which contains single column f3). I
> >> don't get which internal restriction forces it to fail, really, but ok:
> >> there is restriction on COLUMNS LIST and it must be obeyed.
> >>
> >> But in second case you pass EXPRESSION, and I don't think same restriction
> >> should be applied.
> >>
> >> More over, if you look into comments on restriction on GENERATED columns
> >> [2] [3], you will find this restriction is because of nature of STORED
> >> generated columns, and it doesn't bound to VIRTUAL ones. Indeed, there is
> >> suggestion for future to treat GENERATED VIRTUAL columns as expressions.
> >>
> >
> > hi.
> > you already pointed out the problem.
> > As of now GENERATED VIRTUAL columns are not supported as partition keys,
> > therefore we need to disallow generated columns being referenced in
> > the partition key in any representation.
>
> I don't see why "we need to disallow". There are no fundamental reasons.
>
> May be it is better to allow GENERATED VIRTUAL columns? But still forbid
> GENERATED STORED columns because there are reasons for.
>

hi.

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.

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)));

the patch still works, i added this to commitfest
(https://commitfest.postgresql.org/patch/5989)
for tracking purposes.