On 2021-02-04 01:17, Zhihong Yu wrote:
Hi, + if (attribute->attgenerated && !childatt->attgenerated) + ereport(ERROR, ... + if (attribute->attgenerated && childatt->attgenerated) + {Looks like for the second if statement, checking attribute->attgenerated should be enough (due to the check from the first if statement).
Thanks for taking a look. I figured the way I wrote it makes it easier to move the code around or insert other code in the future and doesn't make it so tightly coupled.
Anyway, I figured out how to take account of generation expressions with different column orders. I used the same approach that we use for check constraints. The attached patch is good to go from my perspective.
-- Peter Eisentraut 2ndQuadrant, an EDB company https://www.2ndquadrant.com/
From 3079ddc7e1fdf663bc0e59789c9ceac4eea8fd03 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Fri, 5 Feb 2021 15:14:38 +0100 Subject: [PATCH v2] Fix ALTER TABLE / INHERIT with generated columns When running ALTER TABLE t2 INHERIT t1, we must check that columns in t2 that correspond to a generated column in t1 are also generated and have the same generation expression. Otherwise, this would allow creating setups that a normal CREATE TABLE sequence would not allow. Discussion: https://www.postgresql.org/message-id/22de27f6-7096-8d96-4619-7b882932c...@2ndquadrant.com --- src/backend/commands/tablecmds.c | 60 +++++++++++++++++++++++++ src/test/regress/expected/generated.out | 21 +++++++++ src/test/regress/sql/generated.sql | 15 +++++++ 3 files changed, 96 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 420991e315..bff3c65582 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13963,6 +13963,66 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel) errmsg("column \"%s\" in child table must be marked NOT NULL", attributeName))); + /* + * If parent column generated, child column must be, too. + */ + if (attribute->attgenerated && !childatt->attgenerated) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("column \"%s\" in child table must be a generated column", + attributeName))); + + /* + * Check that both generation expressions match. + * + * The test we apply is to see whether they reverse-compile to the + * same source string. This insulates us from issues like whether + * attributes have the same physical column numbers in parent and + * child relations. (See also constraints_equivalent().) + */ + if (attribute->attgenerated && childatt->attgenerated) + { + TupleConstr *child_constr = child_rel->rd_att->constr; + TupleConstr *parent_constr = parent_rel->rd_att->constr; + char *child_expr = NULL; + char *parent_expr = NULL; + + Assert(child_constr != NULL); + Assert(parent_constr != NULL); + + for (int i = 0; i < child_constr->num_defval; i++) + { + if (child_constr->defval[i].adnum == childatt->attnum) + { + child_expr = + TextDatumGetCString(DirectFunctionCall2(pg_get_expr, + CStringGetTextDatum(child_constr->defval[i].adbin), + ObjectIdGetDatum(child_rel->rd_id))); + break; + } + } + Assert(child_expr != NULL); + + for (int i = 0; i < parent_constr->num_defval; i++) + { + if (parent_constr->defval[i].adnum == attribute->attnum) + { + parent_expr = + TextDatumGetCString(DirectFunctionCall2(pg_get_expr, + CStringGetTextDatum(parent_constr->defval[i].adbin), + ObjectIdGetDatum(parent_rel->rd_id))); + break; + } + } + Assert(parent_expr != NULL); + + if (strcmp(child_expr, parent_expr) != 0) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("column \"%s\" in child table has a conflicting generation expression", + attributeName))); + } + /* * OK, bump the child column's inheritance count. (If we fail * later on, this change will just roll back.) diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out index ca721d38bf..675773f0c1 100644 --- a/src/test/regress/expected/generated.out +++ b/src/test/regress/expected/generated.out @@ -281,6 +281,17 @@ SELECT * FROM gtest_normal; 2 | 4 (2 rows) +CREATE TABLE gtest_normal_child2 (a int, b int GENERATED ALWAYS AS (a * 3) STORED); +ALTER TABLE gtest_normal_child2 INHERIT gtest_normal; +INSERT INTO gtest_normal_child2 (a) VALUES (3); +SELECT * FROM gtest_normal; + a | b +---+--- + 1 | + 2 | 4 + 3 | 9 +(3 rows) + -- test inheritance mismatches between parent and child CREATE TABLE gtestx (x int, b int GENERATED ALWAYS AS (a * 22) STORED) INHERITS (gtest1); -- error NOTICE: merging column "b" with inherited definition @@ -292,6 +303,16 @@ ERROR: column "b" inherits from generated column but specifies default CREATE TABLE gtestx (x int, b int GENERATED ALWAYS AS IDENTITY) INHERITS (gtest1); -- error NOTICE: merging column "b" with inherited definition ERROR: column "b" inherits from generated column but specifies identity +CREATE TABLE gtestxx_1 (a int NOT NULL, b int); +ALTER TABLE gtestxx_1 INHERIT gtest1; -- error +ERROR: column "b" in child table must be a generated column +CREATE TABLE gtestxx_2 (a int NOT NULL, b int GENERATED ALWAYS AS (a * 22) STORED); +ALTER TABLE gtestxx_2 INHERIT gtest1; -- error +ERROR: column "b" in child table has a conflicting generation expression +CREATE TABLE gtestxx_3 (a int NOT NULL, b int GENERATED ALWAYS AS (a * 2) STORED); +ALTER TABLE gtestxx_3 INHERIT gtest1; -- ok +CREATE TABLE gtestxx_4 (b int GENERATED ALWAYS AS (a * 2) STORED, a int NOT NULL); +ALTER TABLE gtestxx_4 INHERIT gtest1; -- ok -- test multiple inheritance mismatches CREATE TABLE gtesty (x int, b int); CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty); -- error diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql index bd2b0bfaaa..19e6e8c39b 100644 --- a/src/test/regress/sql/generated.sql +++ b/src/test/regress/sql/generated.sql @@ -113,11 +113,26 @@ CREATE TABLE gtest_normal_child (a int, b int GENERATED ALWAYS AS (a * 2) STORED INSERT INTO gtest_normal_child (a) VALUES (2); SELECT * FROM gtest_normal; +CREATE TABLE gtest_normal_child2 (a int, b int GENERATED ALWAYS AS (a * 3) STORED); +ALTER TABLE gtest_normal_child2 INHERIT gtest_normal; +INSERT INTO gtest_normal_child2 (a) VALUES (3); +SELECT * FROM gtest_normal; + -- test inheritance mismatches between parent and child CREATE TABLE gtestx (x int, b int GENERATED ALWAYS AS (a * 22) STORED) INHERITS (gtest1); -- error CREATE TABLE gtestx (x int, b int DEFAULT 10) INHERITS (gtest1); -- error CREATE TABLE gtestx (x int, b int GENERATED ALWAYS AS IDENTITY) INHERITS (gtest1); -- error +CREATE TABLE gtestxx_1 (a int NOT NULL, b int); +ALTER TABLE gtestxx_1 INHERIT gtest1; -- error +CREATE TABLE gtestxx_2 (a int NOT NULL, b int GENERATED ALWAYS AS (a * 22) STORED); +ALTER TABLE gtestxx_2 INHERIT gtest1; -- error +CREATE TABLE gtestxx_3 (a int NOT NULL, b int GENERATED ALWAYS AS (a * 2) STORED); +ALTER TABLE gtestxx_3 INHERIT gtest1; -- ok +CREATE TABLE gtestxx_4 (b int GENERATED ALWAYS AS (a * 2) STORED, a int NOT NULL); +ALTER TABLE gtestxx_4 INHERIT gtest1; -- ok + + -- test multiple inheritance mismatches CREATE TABLE gtesty (x int, b int); CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty); -- error base-commit: c444472af5c202067a9ecb0ff8df7370fb1ea8f4 -- 2.30.0
