On Mon, Apr 21, 2025 at 4:02 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
>
>
>
> On 2025/04/21 11:30, jian he wrote:
> > hi.
> > While trying to make the virtual generated column be part of the partition 
> > key,
> > I found this bug.
>
> I haven't looked at the patch in detail yet, but when I applied it
> and ran the regression tests with RELCACHE_FORCE_RELEASE and
> CATCACHE_FORCE_RELEASE enabled, the tests failed with the following diff:
>
> ----------------------------
> ========= Contents of ./src/test/regress/regression.diffs
> diff -U3 
> /home/runner/work/postgresql/postgresql/src/test/regress/expected/create_table.out
>  
> /home/runner/work/postgresql/postgresql/src/test/regress/results/create_table.out
> --- 
> /home/runner/work/postgresql/postgresql/src/test/regress/expected/create_table.out
>   2025-04-21 07:32:03.731119788 +0000
> +++ 
> /home/runner/work/postgresql/postgresql/src/test/regress/results/create_table.out
>    2025-04-21 07:38:31.358134750 +0000
> @@ -810,8 +810,13 @@
>   LINE 1: ...TITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c);
>                                                                       ^
>   CREATE TABLE part_c PARTITION OF parted (b WITH OPTIONS NOT NULL DEFAULT 0) 
> FOR VALUES IN ('c') PARTITION BY RANGE ((b));
> +ERROR:  cannot use generated column in partition key
> +LINE 1: ...ULL DEFAULT 0) FOR VALUES IN ('c') PARTITION BY RANGE ((b));
> +                                                                  ^
> +DETAIL:  Column "b" is a generated column.
>   -- create a level-2 partition
>   CREATE TABLE part_c_1_10 PARTITION OF part_c FOR VALUES FROM (1) TO (10);
> +ERROR:  relation "part_c" does not exist
>   -- check that NOT NULL and default value are inherited correctly
>   create table parted_notnull_inh_test (a int default 1, b int not null 
> default 0) partition by list (a);
>   create table parted_notnull_inh_test1 partition of parted_notnull_inh_test 
> (a not null, b default 1) for values in (1);
> @@ -871,30 +876,8 @@
>
>   -- Both partition bound and partition key in describe output
>   \d+ part_c
> -                             Partitioned table "public.part_c"
> - Column |  Type   | Collation | Nullable | Default | Storage  | Stats target 
> | Description
> ---------+---------+-----------+----------+---------+----------+--------------+-------------
> - a      | text    |           |          |         | extended |              
> |
> - b      | integer |           | not null | 0       | plain    |              
> |
> -Partition of: parted FOR VALUES IN ('c')
> -Partition constraint: ((a IS NOT NULL) AND (a = 'c'::text))
> -Partition key: RANGE (b)
> -Not-null constraints:
> -    "part_c_b_not_null" NOT NULL "b" (local, inherited)
> -Partitions: part_c_1_10 FOR VALUES FROM (1) TO (10)
> -
>   -- a level-2 partition's constraint will include the parent's expressions
>   \d+ part_c_1_10
> -                                Table "public.part_c_1_10"
> - Column |  Type   | Collation | Nullable | Default | Storage  | Stats target 
> | Description
> ---------+---------+-----------+----------+---------+----------+--------------+-------------
> - a      | text    |           |          |         | extended |              
> |
> - b      | integer |           | not null | 0       | plain    |              
> |
> -Partition of: part_c FOR VALUES FROM (1) TO (10)
> -Partition constraint: ((a IS NOT NULL) AND (a = 'c'::text) AND (b IS NOT 
> NULL) AND (b >= 1) AND (b < 10))
> -Not-null constraints:
> -    "part_c_b_not_null" NOT NULL "b" (inherited)
> -
>   -- Show partition count in the parent's describe output
>   -- Tempted to include \d+ output listing partitions with bound info but
>   -- output could vary depending on the order in which partition oids are
> @@ -906,7 +889,7 @@
>    a      | text    |           |          |
>    b      | integer |           | not null | 0
>   Partition key: LIST (a)
> -Number of partitions: 3 (Use \d+ to list them.)
> +Number of partitions: 2 (Use \d+ to list them.)
>
>   \d hash_parted
>         Partitioned table "public.hash_parted"

Thanks for pointing it out.

i think it's related to my silly mistake:
                if (TupleDescAttr(RelationGetDescr(rel),
var->varattno)->attgenerated)
should be
                if (TupleDescAttr(RelationGetDescr(rel), var->varattno
- 1)->attgenerated)

Feel free to test it again.
From 0607e2e3f89e8a30ee9233f1ec253542936a08fd Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Mon, 21 Apr 2025 17:09:27 +0800
Subject: [PATCH v2 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));

discussion: https://postgr.es/m/CACJufxF=wdgthxsaqr9thyusfx_1_t9e6n8te3b8eqxcvov...@mail.gmail.com
---
 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..721734d338d 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 - 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), 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

Reply via email to