Re: bug: virtual generated column can be partition key - Mailing list pgsql-hackers

From jian he
Subject Re: bug: virtual generated column can be partition key
Date
Msg-id CACJufxELB=ea61V6DskP3_zXd5CWX-MF_vQZF4tHKHPVcGGs9Q@mail.gmail.com
Whole thread Raw
In response to Re: bug: virtual generated column can be partition key  (Yura Sokolov <y.sokolov@postgrespro.ru>)
List pgsql-hackers
On Tue, May 6, 2025 at 6:49 PM Yura Sokolov <y.sokolov@postgrespro.ru> wrote:
>
> 06.05.2025 13:31, jian he пишет:
> > On Tue, May 6, 2025 at 5:57 PM Yura Sokolov <y.sokolov@postgrespro.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.



pgsql-hackers by date:

Previous
From: 邱宇航
Date:
Subject: Re: Memory leak of SMgrRelation object on standby
Next
From: Stepan Neretin
Date:
Subject: Re: Fixes a trivial bug in dumped parse/query/plan trees