From d59ecf25468bfd10cc7ddf941faf2fb1092cb01c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sat, 11 Jan 2020 07:33:49 +0100 Subject: [PATCH v3] ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION Add an ALTER TABLE subcommand for dropping the generated property from a column, per SQL standard. Reviewed-by: Sergei Kornilov Discussion: https://www.postgresql.org/message-id/flat/2f7f1d9c-946e-0453-d841-4f38eb9d69b6%402ndquadrant.com --- doc/src/sgml/ref/alter_table.sgml | 18 +++ src/backend/catalog/sql_features.txt | 2 +- src/backend/commands/tablecmds.c | 161 +++++++++++++++++++++++- src/backend/parser/gram.y | 20 ++- src/bin/psql/tab-complete.c | 2 +- src/include/nodes/parsenodes.h | 1 + src/include/parser/kwlist.h | 1 + src/test/regress/expected/generated.out | 87 +++++++++++++ src/test/regress/sql/generated.sql | 38 ++++++ 9 files changed, 326 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 8403c797e2..4bf449587c 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -46,6 +46,7 @@ ALTER [ COLUMN ] column_name SET DEFAULT expression ALTER [ COLUMN ] column_name DROP DEFAULT ALTER [ COLUMN ] column_name { SET | DROP } NOT NULL + ALTER [ COLUMN ] column_name DROP EXPRESSION [ IF EXISTS ] ALTER [ COLUMN ] column_name ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY [ ( sequence_options ) ] ALTER [ COLUMN ] column_name { SET GENERATED { ALWAYS | BY DEFAULT } | SET sequence_option | RESTART [ [ WITH ] restart ] } [...] ALTER [ COLUMN ] column_name DROP IDENTITY [ IF EXISTS ] @@ -241,6 +242,23 @@ Description + + DROP EXPRESSION [ IF EXISTS ] + + + This form turns a stored generated column into a normal base column. + Existing data in the columns is retained, but future changes will no + longer apply the generation expression. + + + + If DROP EXPRESSION IF EXISTS is specified and the + column is not a stored generated column, no error is thrown. In this + case a notice is issued instead. + + + + ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY SET GENERATED { ALWAYS | BY DEFAULT } diff --git a/src/backend/catalog/sql_features.txt b/src/backend/catalog/sql_features.txt index ab3e381cff..9f840ddfd2 100644 --- a/src/backend/catalog/sql_features.txt +++ b/src/backend/catalog/sql_features.txt @@ -252,7 +252,7 @@ F381 Extended schema manipulation 03 ALTER TABLE statement: DROP CONSTRAINT clau F382 Alter column data type YES F383 Set column not null clause YES F384 Drop identity property clause YES -F385 Drop column generation expression clause NO +F385 Drop column generation expression clause YES F386 Set identity column generation clause YES F391 Long identifiers YES F392 Unicode escapes in identifiers YES diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 421bc28727..2ec3fc5014 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -388,6 +388,8 @@ static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName, static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmode); static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode); +static void ATPrepDropExpression(Relation rel, AlterTableCmd *cmd, bool recursing); +static ObjectAddress ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode); static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode); static ObjectAddress ATExecSetOptions(Relation rel, const char *colName, @@ -3672,6 +3674,7 @@ AlterTableGetLockLevel(List *cmds) case AT_AddIdentity: case AT_DropIdentity: case AT_SetIdentity: + case AT_DropExpression: cmd_lockmode = AccessExclusiveLock; break; @@ -3946,6 +3949,12 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, /* No command-specific prep needed */ pass = AT_PASS_COL_ATTRS; break; + case AT_DropExpression: /* ALTER COLUMN DROP EXPRESSION */ + ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATPrepDropExpression(rel, cmd, recursing); + pass = AT_PASS_DROP; + break; case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE); ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); @@ -4265,6 +4274,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, case AT_CheckNotNull: /* check column is already marked NOT NULL */ ATExecCheckNotNull(tab, rel, cmd->name, lockmode); break; + case AT_DropExpression: + address = ATExecDropExpression(rel, cmd->name, cmd->missing_ok, lockmode); + break; case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */ address = ATExecSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode); break; @@ -6457,7 +6469,9 @@ ATExecColumnDefault(Relation rel, const char *colName, ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("column \"%s\" of relation \"%s\" is a generated column", - colName, RelationGetRelationName(rel)))); + colName, RelationGetRelationName(rel)), + newDefault || TupleDescAttr(tupdesc, attnum - 1)->attgenerated != ATTRIBUTE_GENERATED_STORED ? 0 : + errhint("Use ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION instead."))); /* * Remove any old default for the column. We use RESTRICT here for @@ -6725,6 +6739,151 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE return address; } +/* + * ALTER TABLE ALTER COLUMN DROP EXPRESSION + */ +static void +ATPrepDropExpression(Relation rel, AlterTableCmd *cmd, bool recursing) +{ + /* + * Cannot drop generation expression from inherited columns. + */ + if (!recursing) + { + HeapTuple tuple; + Form_pg_attribute attTup; + + tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), cmd->name); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + cmd->name, RelationGetRelationName(rel)))); + + attTup = (Form_pg_attribute) GETSTRUCT(tuple); + + if (attTup->attinhcount > 0) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot drop generation expression from inherited column"))); + } +} + +/* + * Return the address of the affected column. + */ +static ObjectAddress +ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode) +{ + HeapTuple tuple; + Form_pg_attribute attTup; + AttrNumber attnum; + Relation attrelation; + ObjectAddress address; + + attrelation = table_open(AttributeRelationId, RowExclusiveLock); + tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + colName, RelationGetRelationName(rel)))); + + attTup = (Form_pg_attribute) GETSTRUCT(tuple); + attnum = attTup->attnum; + + if (attnum <= 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter system column \"%s\"", + colName))); + + if (attTup->attgenerated != ATTRIBUTE_GENERATED_STORED) + { + if (!missing_ok) + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("column \"%s\" of relation \"%s\" is not a stored generated column", + colName, RelationGetRelationName(rel)))); + else + { + ereport(NOTICE, + (errmsg("column \"%s\" of relation \"%s\" is not a stored generated column, skipping", + colName, RelationGetRelationName(rel)))); + heap_freetuple(tuple); + table_close(attrelation, RowExclusiveLock); + return InvalidObjectAddress; + } + } + + attTup->attgenerated = '\0'; + CatalogTupleUpdate(attrelation, &tuple->t_self, tuple); + + InvokeObjectPostAlterHook(RelationRelationId, + RelationGetRelid(rel), + attTup->attnum); + ObjectAddressSubSet(address, RelationRelationId, + RelationGetRelid(rel), attnum); + heap_freetuple(tuple); + + table_close(attrelation, RowExclusiveLock); + + CommandCounterIncrement(); + + RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, false, false); + + /* + * Remove all dependencies of this (formerly generated) column on other + * columns in the same table. (See StoreAttrDefault() for which + * dependencies are created.) We don't expect there to be dependencies + * between columns of the same table for other reasons, so it's okay to + * remove all of them. + */ + { + Relation depRel; + ScanKeyData key[3]; + SysScanDesc scan; + HeapTuple tup; + + depRel = table_open(DependRelationId, RowExclusiveLock); + + ScanKeyInit(&key[0], + Anum_pg_depend_classid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationRelationId)); + ScanKeyInit(&key[1], + Anum_pg_depend_objid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + ScanKeyInit(&key[2], + Anum_pg_depend_objsubid, + BTEqualStrategyNumber, F_INT4EQ, + Int32GetDatum(attnum)); + + scan = systable_beginscan(depRel, DependDependerIndexId, true, + NULL, 3, key); + + while (HeapTupleIsValid(tup = systable_getnext(scan))) + { + Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup); + + if (depform->refclassid == RelationRelationId && + depform->refobjid == RelationGetRelid(rel) && + depform->refobjsubid != 0 && + depform->deptype == DEPENDENCY_AUTO) + { + CatalogTupleDelete(depRel, &tup->t_self); + } + } + + systable_endscan(scan); + + table_close(depRel, RowExclusiveLock); + } + + return address; +} + /* * ALTER TABLE ALTER COLUMN SET STATISTICS * diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index ad5be902b0..bb1e499d52 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -636,7 +636,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); DOUBLE_P DROP EACH ELSE ENABLE_P ENCODING ENCRYPTED END_P ENUM_P ESCAPE EVENT EXCEPT - EXCLUDE EXCLUDING EXCLUSIVE EXECUTE EXISTS EXPLAIN + EXCLUDE EXCLUDING EXCLUSIVE EXECUTE EXISTS EXPLAIN EXPRESSION EXTENSION EXTERNAL EXTRACT FALSE_P FAMILY FETCH FILTER FIRST_P FLOAT_P FOLLOWING FOR @@ -2126,6 +2126,23 @@ alter_table_cmd: n->name = $3; $$ = (Node *)n; } + /* ALTER TABLE ALTER [COLUMN] DROP EXPRESSION */ + | ALTER opt_column ColId DROP EXPRESSION + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_DropExpression; + n->name = $3; + $$ = (Node *)n; + } + /* ALTER TABLE ALTER [COLUMN] DROP EXPRESSION IF EXISTS */ + | ALTER opt_column ColId DROP EXPRESSION IF_P EXISTS + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_DropExpression; + n->name = $3; + n->missing_ok = true; + $$ = (Node *)n; + } /* ALTER TABLE ALTER [COLUMN] SET STATISTICS */ | ALTER opt_column ColId SET STATISTICS SignedIconst { @@ -15193,6 +15210,7 @@ unreserved_keyword: | EXCLUSIVE | EXECUTE | EXPLAIN + | EXPRESSION | EXTENSION | EXTERNAL | FAMILY diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 2fd88866c9..b52396c17a 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2015,7 +2015,7 @@ psql_completion(const char *text, int start, int end) /* ALTER TABLE ALTER [COLUMN] DROP */ else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "DROP") || Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "DROP")) - COMPLETE_WITH("DEFAULT", "IDENTITY", "NOT NULL"); + COMPLETE_WITH("DEFAULT", "EXPRESSION", "IDENTITY", "NOT NULL"); else if (Matches("ALTER", "TABLE", MatchAny, "CLUSTER")) COMPLETE_WITH("ON"); else if (Matches("ALTER", "TABLE", MatchAny, "CLUSTER", "ON")) diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index cdfa0568f7..28d837b8fa 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1785,6 +1785,7 @@ typedef enum AlterTableType AT_ColumnDefault, /* alter column default */ AT_DropNotNull, /* alter column drop not null */ AT_SetNotNull, /* alter column set not null */ + AT_DropExpression, /* alter column drop expression */ AT_CheckNotNull, /* check column is already marked not null */ AT_SetStatistics, /* alter column set statistics */ AT_SetOptions, /* alter column set ( options ) */ diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index 0fe4e6cb20..3c1c232d37 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -155,6 +155,7 @@ PG_KEYWORD("exclusive", EXCLUSIVE, UNRESERVED_KEYWORD) PG_KEYWORD("execute", EXECUTE, UNRESERVED_KEYWORD) PG_KEYWORD("exists", EXISTS, COL_NAME_KEYWORD) PG_KEYWORD("explain", EXPLAIN, UNRESERVED_KEYWORD) +PG_KEYWORD("expression", EXPRESSION, UNRESERVED_KEYWORD) PG_KEYWORD("extension", EXTENSION, UNRESERVED_KEYWORD) PG_KEYWORD("external", EXTERNAL, UNRESERVED_KEYWORD) PG_KEYWORD("extract", EXTRACT, COL_NAME_KEYWORD) diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out index 8cffef0477..a6d3670af4 100644 --- a/src/test/regress/expected/generated.out +++ b/src/test/regress/expected/generated.out @@ -648,6 +648,7 @@ ALTER TABLE gtest27 ALTER COLUMN x TYPE boolean USING x <> 0; -- error ERROR: generation expression for column "x" cannot be cast automatically to type boolean ALTER TABLE gtest27 ALTER COLUMN x DROP DEFAULT; -- error ERROR: column "x" of relation "gtest27" is a generated column +HINT: Use ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION instead. -- It's possible to alter the column types this way: ALTER TABLE gtest27 DROP COLUMN x, @@ -683,6 +684,92 @@ SELECT * FROM gtest27; 4 | 11 | 30 (2 rows) +-- ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION +CREATE TABLE gtest29 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +INSERT INTO gtest29 (a) VALUES (3), (4); +ALTER TABLE gtest29 ALTER COLUMN a DROP EXPRESSION; -- error +ERROR: column "a" of relation "gtest29" is not a stored generated column +ALTER TABLE gtest29 ALTER COLUMN a DROP EXPRESSION IF EXISTS; -- notice +NOTICE: column "a" of relation "gtest29" is not a stored generated column, skipping +ALTER TABLE gtest29 ALTER COLUMN b DROP EXPRESSION; +INSERT INTO gtest29 (a) VALUES (5); +INSERT INTO gtest29 (a, b) VALUES (6, 66); +SELECT * FROM gtest29; + a | b +---+---- + 3 | 6 + 4 | 8 + 5 | + 6 | 66 +(4 rows) + +\d gtest29 + Table "public.gtest29" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | + +-- check that dependencies between columns have also been removed +ALTER TABLE gtest29 DROP COLUMN a; -- should not drop b +\d gtest29 + Table "public.gtest29" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + b | integer | | | + +-- with inheritance +CREATE TABLE gtest30 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest30_1 () INHERITS (gtest30); +ALTER TABLE gtest30 ALTER COLUMN b DROP EXPRESSION; +\d gtest30 + Table "public.gtest30" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Number of child tables: 1 (Use \d+ to list them.) + +\d gtest30_1 + Table "public.gtest30_1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Inherits: gtest30 + +DROP TABLE gtest30 CASCADE; +NOTICE: drop cascades to table gtest30_1 +CREATE TABLE gtest30 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest30_1 () INHERITS (gtest30); +ALTER TABLE ONLY gtest30 ALTER COLUMN b DROP EXPRESSION; +\d gtest30 + Table "public.gtest30" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Number of child tables: 1 (Use \d+ to list them.) + +\d gtest30_1 + Table "public.gtest30_1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+------------------------------------ + a | integer | | | + b | integer | | | generated always as (a * 2) stored +Inherits: gtest30 + +ALTER TABLE gtest30_1 ALTER COLUMN b DROP EXPRESSION; -- error +ERROR: cannot drop generation expression from inherited column -- triggers CREATE TABLE gtest26 ( a int PRIMARY KEY, diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql index ff5c8607de..f0e6a22dac 100644 --- a/src/test/regress/sql/generated.sql +++ b/src/test/regress/sql/generated.sql @@ -351,6 +351,44 @@ CREATE TABLE gtest27 ( \d gtest27 SELECT * FROM gtest27; +-- ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION +CREATE TABLE gtest29 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +INSERT INTO gtest29 (a) VALUES (3), (4); +ALTER TABLE gtest29 ALTER COLUMN a DROP EXPRESSION; -- error +ALTER TABLE gtest29 ALTER COLUMN a DROP EXPRESSION IF EXISTS; -- notice +ALTER TABLE gtest29 ALTER COLUMN b DROP EXPRESSION; +INSERT INTO gtest29 (a) VALUES (5); +INSERT INTO gtest29 (a, b) VALUES (6, 66); +SELECT * FROM gtest29; +\d gtest29 + +-- check that dependencies between columns have also been removed +ALTER TABLE gtest29 DROP COLUMN a; -- should not drop b +\d gtest29 + +-- with inheritance +CREATE TABLE gtest30 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest30_1 () INHERITS (gtest30); +ALTER TABLE gtest30 ALTER COLUMN b DROP EXPRESSION; +\d gtest30 +\d gtest30_1 +DROP TABLE gtest30 CASCADE; +CREATE TABLE gtest30 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest30_1 () INHERITS (gtest30); +ALTER TABLE ONLY gtest30 ALTER COLUMN b DROP EXPRESSION; +\d gtest30 +\d gtest30_1 +ALTER TABLE gtest30_1 ALTER COLUMN b DROP EXPRESSION; -- error + -- triggers CREATE TABLE gtest26 ( a int PRIMARY KEY, -- 2.24.1