Subject: bug: virtual generated column can be partition key

From: jian he <[email-masked]>
Date: Mon, 21 Apr 2025 10:30:01 +0800

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));
From 10f8c6a1c59a31c2ba6d77d69fdc740e094d9cd3 Mon Sep 17 00:00:00 2001
From: jian he <[email-masked]>
Date: Mon, 21 Apr 2025 10:28:26 +0800
Subject: [PATCH v1 1/1] virtual generated column can be partition key

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));
---
 src/backend/commands/tablecmds.c                | 12 +++++++++++-
 src/test/regress/expected/generated_stored.out  |  5 +++++
 src/test/regress/expected/generated_virtual.out |  5 +++++
 src/test/regress/sql/generated_stored.sql       |  1 +
 src/test/regress/sql/generated_virtual.sql      |  1 +
 5 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 265b1c397fb..4b96cd73ba4 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19794,11 +19794,21 @@ ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNu
 			if (IsA(expr, Var) &&
 				((Var *) expr)->varattno > 0)
 			{
+				Var		   *var = (Var *) expr;
+
+				if (TupleDescAttr(RelationGetDescr(rel), var->varattno)->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), var->varattno, false)),
+							parser_errposition(pstate, pelem->location));
+
 				/*
 				 * User wrote "(column)" or "(column COLLATE something)".
 				 * Treat it like simple attribute anyway.
 				 */
-				partattrs[attn] = ((Var *) expr)->varattno;
+				partattrs[attn] = var->varattno;
 			}
 			else
 			{
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index 16de30ab191..72f39f2f0c1 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -1074,6 +1074,11 @@ ERROR:  cannot use generated column in partition key
 LINE 1: ...ENERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE (f3);
                                                                    ^
 DETAIL:  Column "f3" is a generated column.
+CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE ((f3));
+ERROR:  cannot use generated column in partition key
+LINE 1: ...ERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE ((f3));
+                                                                 ^
+DETAIL:  Column "f3" is a generated column.
 CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE ((f3 * 3));
 ERROR:  cannot use generated column in partition key
 LINE 1: ...ED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE ((f3 * 3));
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 6300e7c1d96..2a6dc84b8ca 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1027,6 +1027,11 @@ ERROR:  cannot use generated column in partition key
 LINE 1: ...NERATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE (f3);
                                                                    ^
 DETAIL:  Column "f3" is a generated column.
+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 1: ...RATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE ((f3));
+                                                                 ^
+DETAIL:  Column "f3" is a generated column.
 CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE ((f3 * 3));
 ERROR:  cannot use generated column in partition key
 LINE 1: ...D ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE ((f3 * 3));
diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index 4ec155f2da9..c9427c2d4d8 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -500,6 +500,7 @@ SELECT tableoid::regclass, * FROM gtest_parent ORDER BY 1, 2, 3;
 
 -- generated columns in partition key (not allowed)
 CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE (f3);
+CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE ((f3));
 CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) STORED) PARTITION BY RANGE ((f3 * 3));
 
 -- ALTER TABLE ... ADD COLUMN
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index b4eedeee2fb..652057bd707 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -534,6 +534,7 @@ SELECT tableoid::regclass, * FROM gtest_parent ORDER BY 1, 2, 3;
 
 -- generated columns in partition key (not allowed)
 CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE (f3);
+CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE ((f3));
 CREATE TABLE gtest_part_key (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED ALWAYS AS (f2 * 2) VIRTUAL) PARTITION BY RANGE ((f3 * 3));
 
 -- ALTER TABLE ... ADD COLUMN
-- 
2.34.1

---

From: Yura Sokolov <[email-masked]>
Date: Tue, 6 May 2025 12:57:29 +0300

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

So, if you want to force some restriction, you should force it only against
STORED columns, but not VIRTUAL ones.
Of cause, if VIRTUAL column depends on STORED one, it is still should be
forbidden.

Certainly, it is my opinion and I could be mistaken somewhere.
For example, you may say "it is too hard to check dependency of VIRTUAL
column at the moment, so it is simpler to forbid them as well". But then it
should be clearly stated in commit messages and code comments.

[1]
https://www.postgresql.org/docs/17/ddl-partitioning.html#DDL-PARTITIONING-DECLARATIVE
[2]
https://github.com/postgres/postgres/blob/caa76b91a60681dff0bf193b64d4dcdc1014e036/src/backend/commands/tablecmds.c#L19735-L19741
[3]
https://github.com/postgres/postgres/blob/caa76b91a60681dff0bf193b64d4dcdc1014e036/src/backend/commands/tablecmds.c#L19821-L19828

-- 
regards
Yura Sokolov aka funny-falcon

---

From: jian he <[email-masked]>
Date: Tue, 6 May 2025 18:31:38 +0800

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

currently in a matser:
CREATE TABLE xx (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED
ALWAYS AS (f2 * 2) virtual) PARTITION BY RANGE (f3);
will yield error, however
CREATE TABLE xx (f1 date NOT NULL, f2 bigint, f3 bigint GENERATED
ALWAYS AS (f2 * 2) virtual) PARTITION BY RANGE ((f3));
will not.
but these two form essentially the same thing.

"PARTITION BY RANGE ((f3));"  and "PARTITION BY RANGE ((whole_row));"
are the corner cases we are trying to catch.

you may also see ComputePartitionAttrs.

---

From: Yura Sokolov <[email-masked]>
Date: Tue, 6 May 2025 13:49:26 +0300

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

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

---

From: jian he <[email-masked]>
Date: Mon, 25 Aug 2025 10:57:50 +0800

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.

---

From: Ashutosh Bapat <[email-masked]>
Date: Fri, 29 Aug 2025 19:24:20 +0530

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

>                              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