On 2020-04-23 08:35, Masahiko Sawada wrote:
After investigating this issue, I think that current DDLs regarding
inherited tables and generated columns seem not to work fine.
Right, there were a number of combinations that were not properly
handled. The attached patch should fix them all. It's made against
PG12 but also works on master. See contained commit message and
documentation for details.
(This does not touch the issues with pg_dump, but it helps clarify the
cases that pg_dump needs to handle.)
We can make an inherited table have the same column having a different
generation expression as follows:
=# create table p1 (a int, b int generated always as (a + 1) stored);
=# create table c1 (a int, b int generated always as (a + 2) stored)
inherits(p1);
With my patch, this becomes an error.
But the column on the inherited table has a default value, the column
will be generation expression with a const value:
=# create table p2 (a int, b int generated always as (a + 1) stored);
=# create table c2 (a int, b int default 10) inherits(p2);
=# \d c2
Table "public.c2"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------------------------------
a | integer | | |
b | integer | | | generated always as (10) stored
Inherits: p2
With my patch, this also becomes an error.
Also, CREATE TABLE doesn't support to create a generated column on
inherited table, which is the same name but is a normal column on the
parent table, as follows:
=# create table p3 (a int, b int);
=# create table c3 (a int, b int generated always as (a + 2) stored)
inherits(p3);
ERROR: cannot use column reference in DEFAULT expression
LINE 1: ...reate table c3 (a int, b int generated always as (a + 2) sto...
This is allowed with my patch (which is basically an expanded version of
your patch).
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 2c82a0f77ff676634e27782b147fce9c0089fb78 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 6 May 2020 16:25:54 +0200
Subject: [PATCH] Fix several DDL issues of generated columns versus
inheritance
Several combinations of generated columns and inheritance in CREATE
TABLE were not handled correctly. Specifically:
- Disallow a child column specifying a generation expression if the
parent column is a generated column. The child column definition
must be unadorned and the parent column's generation expression will
be copied.
- Prohibit a child column of a generated parent column specifying
default values or identity.
- Allow a child column of a not-generated parent column specifying
itself as a generated column. This previously did not work, but it
was possible to arrive at the state via other means (involving ALTER
TABLE), so it seems sensible to support it.
Add tests for each case. Also add documentation about the rules
involving generated columns and inheritance.
Discussion:
https://www.postgresql.org/message-id/flat/15830.1575468847%40sss.pgh.pa.us
https://www.postgresql.org/message-id/flat/2678bad1-048f-519a-ef24-b12962f41807%40enterprisedb.com
https://www.postgresql.org/message-id/flat/CAJvUf_u4h0DxkCMCeEKAWCuzGUTnDP-G5iVmSwxLQSXn0_FWNQ%40mail.gmail.com
---
doc/src/sgml/ddl.sgml | 26 +++++++++++
src/backend/commands/tablecmds.c | 61 +++++++++++++++++++++++--
src/test/regress/expected/generated.out | 44 +++++++++++++++++-
src/test/regress/sql/generated.sql | 22 ++++++++-
4 files changed, 146 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index a20e5fb366..bf9f0181ea 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -324,6 +324,32 @@ <title>Generated Columns</title>
linkend="sql-createforeigntable"/> for details.
</para>
</listitem>
+ <listitem>
+ <para>For inheritance:</para>
+ <itemizedlist>
+ <listitem>
+ <para>
+ If a parent column is a generated column, a child column must also be
+ a generated column using the same expression. In the definition of
+ the child column, leave off the <literal>GENERATED</literal> clause,
+ as it will be copied from the parent.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ In case of multiple inheritance, if one parent column is a generated
+ column, then all parent columns must be generated columns and with the
+ same expression.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ If a parent column is not a generated column, a child column may be
+ defined to be a generated column or not.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </listitem>
</itemizedlist>
</para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1a6ccff8f1..673166bca5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2577,12 +2577,55 @@ MergeAttributes(List *schema, List *supers, char
relpersistence,
def->is_local = true;
/* Merge of NOT NULL constraints = OR 'em
together */
def->is_not_null |= newdef->is_not_null;
+
+ /*
+ * Check for conflicts related to generated
columns.
+ *
+ * If the parent column is generated, the child
column must be
+ * unadorned and will be made a generated
column. (We could
+ * in theory allow the child column definition
specifying the
+ * exact same generation expression, but that's
a bit
+ * complicated to implement and doesn't seem
very useful.) We
+ * also check that the child column doesn't
specify a default
+ * value or identity, which matches the rules
for a single
+ * column in parse_util.c.
+ */
+ if (def->generated)
+ {
+ if (newdef->generated)
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
+ errmsg("child
column \"%s\" specifies generation expression",
+
def->colname),
+ errhint("Leave
off the generation expression in the definition of the child table column to
inherit the generation expression from the parent table.")));
+ if (newdef->raw_default &&
!newdef->generated)
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
+ errmsg("column
\"%s\" inherits from generated column but specifies default",
+
def->colname)));
+ if (newdef->identity)
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
+ errmsg("column
\"%s\" inherits from generated column but specifies identity",
+
def->colname)));
+ }
+ /*
+ * If the parent column is not generated, then
take whatever
+ * the child column definition says.
+ */
+ else
+ {
+ if (newdef->generated)
+ def->generated =
newdef->generated;
+ }
+
/* If new def has a default, override previous
default */
if (newdef->raw_default != NULL)
{
def->raw_default = newdef->raw_default;
def->cooked_default =
newdef->cooked_default;
}
+
}
else
{
@@ -2668,11 +2711,19 @@ MergeAttributes(List *schema, List *supers, char
relpersistence,
ColumnDef *def = lfirst(entry);
if (def->cooked_default == &bogus_marker)
- ereport(ERROR,
-
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
- errmsg("column \"%s\" inherits
conflicting default values",
- def->colname),
- errhint("To resolve the
conflict, specify a default explicitly.")));
+ {
+ if (def->generated)
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
+ errmsg("column \"%s\"
inherits conflicting generation expressions",
+
def->colname)));
+ else
+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
+ errmsg("column \"%s\"
inherits conflicting default values",
+
def->colname),
+ errhint("To resolve
the conflict, specify a default explicitly.")));
+ }
}
}
diff --git a/src/test/regress/expected/generated.out
b/src/test/regress/expected/generated.out
index f87c86a8ce..0436773628 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -219,12 +219,54 @@ SELECT * FROM gtest1;
4 | 8
(2 rows)
--- test inheritance mismatch
+CREATE TABLE gtest_normal (a int, b int);
+CREATE TABLE gtest_normal_child (a int, b int GENERATED ALWAYS AS (a * 2)
STORED) INHERITS (gtest_normal);
+NOTICE: merging column "a" with inherited definition
+NOTICE: merging column "b" with inherited definition
+\d gtest_normal_child
+ Table "public.gtest_normal_child"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+------------------------------------
+ a | integer | | |
+ b | integer | | | generated always as (a * 2) stored
+Inherits: gtest_normal
+
+INSERT INTO gtest_normal (a) VALUES (1);
+INSERT INTO gtest_normal_child (a) VALUES (2);
+SELECT * FROM gtest_normal;
+ a | b
+---+---
+ 1 |
+ 2 | 4
+(2 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
+ERROR: child column "b" specifies generation expression
+HINT: Leave off the generation expression in the definition of the child
table column to inherit the generation expression from the parent table.
+CREATE TABLE gtestx (x int, b int DEFAULT 10) INHERITS (gtest1); -- error
+NOTICE: merging column "b" with inherited definition
+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
+-- test multiple inheritance mismatches
CREATE TABLE gtesty (x int, b int);
CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty); -- error
NOTICE: merging multiple inherited definitions of column "b"
ERROR: inherited column "b" has a generation conflict
DROP TABLE gtesty;
+CREATE TABLE gtesty (x int, b int GENERATED ALWAYS AS (x * 22) STORED);
+CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty); -- error
+NOTICE: merging multiple inherited definitions of column "b"
+ERROR: column "b" inherits conflicting generation expressions
+DROP TABLE gtesty;
+CREATE TABLE gtesty (x int, b int DEFAULT 55);
+CREATE TABLE gtest1_2 () INHERITS (gtest0, gtesty); -- error
+NOTICE: merging multiple inherited definitions of column "b"
+ERROR: inherited column "b" has a generation conflict
+DROP TABLE gtesty;
-- test stored update
CREATE TABLE gtest3 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
INSERT INTO gtest3 (a) VALUES (1), (2), (3), (NULL);
diff --git a/src/test/regress/sql/generated.sql
b/src/test/regress/sql/generated.sql
index bdcedbb991..f089f9a74d 100644
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -89,11 +89,31 @@ CREATE TABLE gtest1_1 () INHERITS (gtest1);
SELECT * FROM gtest1_1;
SELECT * FROM gtest1;
--- test inheritance mismatch
+CREATE TABLE gtest_normal (a int, b int);
+CREATE TABLE gtest_normal_child (a int, b int GENERATED ALWAYS AS (a * 2)
STORED) INHERITS (gtest_normal);
+\d gtest_normal_child
+INSERT INTO gtest_normal (a) VALUES (1);
+INSERT INTO gtest_normal_child (a) VALUES (2);
+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
+
+-- test multiple inheritance mismatches
CREATE TABLE gtesty (x int, b int);
CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty); -- error
DROP TABLE gtesty;
+CREATE TABLE gtesty (x int, b int GENERATED ALWAYS AS (x * 22) STORED);
+CREATE TABLE gtest1_2 () INHERITS (gtest1, gtesty); -- error
+DROP TABLE gtesty;
+
+CREATE TABLE gtesty (x int, b int DEFAULT 55);
+CREATE TABLE gtest1_2 () INHERITS (gtest0, gtesty); -- error
+DROP TABLE gtesty;
+
-- test stored update
CREATE TABLE gtest3 (a int, b int GENERATED ALWAYS AS (a * 3) STORED);
INSERT INTO gtest3 (a) VALUES (1), (2), (3), (NULL);
base-commit: 987717d7c7007055ff7fc2ecf3d40e9bdb00e071
--
2.26.2
