pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: 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: Wed, 3 Feb 2021 20:18:33 +0100
Message-ID: <22de27f6-7096-8d96-4619-7b882932ca25@2ndquadrant.com> (raw)
In-Reply-To: <2808494.1611938475@sss.pgh.pa.us>
References: <15830.1575468847@sss.pgh.pa.us>
	<fd559544-274c-f514-d8bf-358d2c539455@2ndquadrant.com>
	<31905.1575491779@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>

On 2021-01-29 17:41, Tom Lane wrote:
> Also, in the example from [2],
> 
> d3=# create table pp1 (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
> CREATE TABLE
> d3=# create table cc1 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
> CREATE TABLE
> d3=# alter table cc1 inherit pp1;
> ALTER TABLE
> 
> pg_dump now omits to dump cc1's generation expression, which seems
> strictly worse than before.  Admittedly, the backend likely ought to
> be rejecting this scenario, but it doesn't do so today.
> 
> [2]https://www.postgresql.org/message-id/661371.1601398006%40sss.pgh.pa.us

Here is a WIP patch to address this.  Probably needs another look for 
column number mapping and all the usual stuff, but the basic idea should 
be okay.

-- 
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/

Attachments:

0001-WIP-Fix-ALTER-TABLE-INHERIT-with-generated-columns.patch (text/plain, 5.7K) download From bd6ee462e5ad4a6514940e7e78d40c4f28f3dc3f Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter@eisentraut.org> Date: Wed, 3 Feb 2021 20:14:05 +0100 Subject: [PATCH] WIP: 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. --- src/backend/commands/tablecmds.c | 49 +++++++++++++++++++++++++ src/test/regress/expected/generated.out | 19 ++++++++++ src/test/regress/sql/generated.sql | 13 +++++++ 3 files changed, 81 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 420991e315..3d4fb4ce7e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13963,6 +13963,55 @@ 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. + */ + if (attribute->attgenerated && childatt->attgenerated) + { + TupleConstr *child_constr = child_rel->rd_att->constr; + TupleConstr *parent_constr = parent_rel->rd_att->constr; + Node *child_expr = NULL; + Node *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 = stringToNode(child_constr->defval[i].adbin); + 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 = stringToNode(parent_constr->defval[i].adbin); + break; + } + } + Assert(parent_expr != NULL); + + if (!equal(child_expr, parent_expr)) + 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..f2ed3864a1 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,14 @@ 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 -- 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..e30dfe649d 100644 --- a/src/test/regress/sql/generated.sql +++ b/src/test/regress/sql/generated.sql @@ -113,11 +113,24 @@ 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 + + -- test multiple inheritance mismatches CREATE TABLE gtesty (x int, b int); CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty); -- error -- 2.30.0

view thread (37+ messages)  latest in thread

Message-ID: <22de27f6-7096-8d96-4619-7b882932ca25@2ndquadrant.com>
Permalink:  ../22de27f6-7096-8d96-4619-7b882932ca25@2ndquadrant.com/
Also on:    postgresql.org/message-id/22de27f6-7096-8d96-4619-7b882932ca25@2ndquadrant.com

 ·  · 

reply

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, 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: <22de27f6-7096-8d96-4619-7b882932ca25@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