From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Zhihong Yu <zyu@yugabyte.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Dumping/restoring fails on inherited generated column
Date: Fri, 5 Feb 2021 15:18:15 +0100
Message-ID: <0dbc982e-fbb3-911e-eb80-130494664940@2ndquadrant.com> (raw)
In-Reply-To: <CALNJ-vS1PtJBk3v73Bq3dM9J46=P3jAcXOLjjtZ7w2fPYsg-7A@mail.gmail.com>
References: <15830.1575468847@sss.pgh.pa.us>
<d6ebcc2e-3e6f-6915-6616-70ef665aed66@2ndquadrant.com>
<24369.1580758361@sss.pgh.pa.us>
<b1c831dd-d520-5e7f-0304-0eeed39c9996@2ndquadrant.com>
<CA+fd4k4cHKUmEyHiPHx0QL3T-DXAP8O1CP-hUvo1WX6RwNC=tA@mail.gmail.com>
<3f272982-0e0d-17a1-3fb6-fedc74197676@2ndquadrant.com>
<c9848634-76c1-3c21-fc19-f1b7eebb640f@2ndquadrant.com>
<3169466.1594841366@sss.pgh.pa.us>
<CA+fd4k4fck6xzsF8mPWeVSZFBLtiA2cQxrPqBaYcoZGc+hZ=JQ@mail.gmail.com>
<CA+fd4k6pLzrZDQsdsxcS06AwGRf1DgwOw84sFq9oXNw+83nB1g@mail.gmail.com>
<e87e2efe-c820-c0d3-f7a0-0789beb78ff1@2ndquadrant.com>
<CAD21AoCfgYKJo3cEhoUr4cKWxmSxi_gdHQDmOJ5XKC-dDc4yBw@mail.gmail.com>
<e8e3f666-9050-1029-42cf-d0ecfdb8902e@2ndquadrant.com>
<0f57d5e9-4112-1ba8-64d8-c02062c1e95d@postgrespro.ru>
<41888c80-e5bd-f9e5-75dc-fb377113168c@2ndquadrant.com>
<2808494.1611938475@sss.pgh.pa.us>
<22de27f6-7096-8d96-4619-7b882932ca25@2ndquadrant.com>
<CALNJ-vS1PtJBk3v73Bq3dM9J46=P3jAcXOLjjtZ7w2fPYsg-7A@mail.gmail.com>
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/Attachments:v2-0001-Fix-ALTER-TABLE-INHERIT-with-generated-columns.patch (text/plain, 6.7K) download
From 3079ddc7e1fdf663bc0e59789c9ceac4eea8fd03 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@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-7b882932ca25@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.cindex 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.outindex 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.sqlindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com, zyu@yugabyte.com, tgl@sss.pgh.pa.us, sawada.mshk@gmail.com, masahiko.sawada@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Dumping/restoring fails on inherited generated column
In-Reply-To: <0dbc982e-fbb3-911e-eb80-130494664940@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox