On 2019-12-25 12:01, Sergei Kornilov wrote:
Patch does not apply to master. Could you rebase?
done
I noticed one bug: create table testdrop (i int, b int, m int GENERATED ALWAYS AS ( i*2) stored); insert into testdrop(i,b) values (3,4); alter table testdrop alter COLUMN m drop expression ; alter table testdrop drop column i; Here is no "m" column anymore. Possible due some forgotten dependency?
fixed -- good catch -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 465312aafd88325a48d934a2c64ca6b9d12dea3f Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Sat, 28 Dec 2019 17:38:12 +0100 Subject: [PATCH v2] ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION Add an ALTER TABLE subcommand for dropping the generated property from a column, per SQL standard. 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 | 131 +++++++++++++++++++++++- 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 | 80 +++++++++++++++ src/test/regress/sql/generated.sql | 32 ++++++ 9 files changed, 283 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 ] <replaceable class="parameter">column_name</replaceable> SET DEFAULT <replaceable class="parameter">expression</replaceable> ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> DROP DEFAULT ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> { SET | DROP } NOT NULL + ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> DROP EXPRESSION [ IF EXISTS ] ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY [ ( <replaceable>sequence_options</replaceable> ) ] ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> { SET GENERATED { ALWAYS | BY DEFAULT } | SET <replaceable>sequence_option</replaceable> | RESTART [ [ WITH ] <replaceable class="parameter">restart</replaceable> ] } [...] ALTER [ COLUMN ] <replaceable class="parameter">column_name</replaceable> DROP IDENTITY [ IF EXISTS ] @@ -241,6 +242,23 @@ <title>Description</title> </listitem> </varlistentry> + <varlistentry> + <term><literal>DROP EXPRESSION [ IF EXISTS ]</literal></term> + <listitem> + <para> + 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. + </para> + + <para> + If <literal>DROP EXPRESSION IF EXISTS</literal> is specified and the + column is not a stored generated column, no error is thrown. In this + case a notice is issued instead. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY</literal></term> <term><literal>SET GENERATED { ALWAYS | BY DEFAULT }</literal></term> 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 5b882f80bf..603779ae9d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -386,6 +386,7 @@ 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 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, @@ -3669,6 +3670,7 @@ AlterTableGetLockLevel(List *cmds) case AT_AddIdentity: case AT_DropIdentity: case AT_SetIdentity: + case AT_DropExpression: cmd_lockmode = AccessExclusiveLock; break; @@ -3943,6 +3945,11 @@ 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); + 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); @@ -4262,6 +4269,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; @@ -6428,7 +6438,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 @@ -6696,6 +6708,123 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE return address; } +/* + * ALTER TABLE ALTER COLUMN DROP EXPRESSION + * + * 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 c5086846de..25e1aa66d9 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 <name> ALTER [COLUMN] <colname> DROP EXPRESSION */ + | ALTER opt_column ColId DROP EXPRESSION + { + AlterTableCmd *n = makeNode(AlterTableCmd); + n->subtype = AT_DropExpression; + n->name = $3; + $$ = (Node *)n; + } + /* ALTER TABLE <name> ALTER [COLUMN] <colname> 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 <name> ALTER [COLUMN] <colname> SET STATISTICS <SignedIconst> */ | 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 5e0db3515d..7e7e6d5636 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2009,7 +2009,7 @@ psql_completion(const char *text, int start, int end) /* ALTER TABLE ALTER [COLUMN] <foo> 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 ff626cbe61..b7aaab69dc 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1764,6 +1764,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 00ace8425e..5bf0c13dca 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 f62c93f468..e03112869b 100644 --- a/src/test/regress/expected/generated.out +++ b/src/test/regress/expected/generated.out @@ -621,6 +621,7 @@ ALTER TABLE gtest27 ALTER COLUMN b TYPE boolean USING b <> 0; -- error ERROR: generation expression for column "b" cannot be cast automatically to type boolean ALTER TABLE gtest27 ALTER COLUMN b DROP DEFAULT; -- error ERROR: column "b" of relation "gtest27" is a generated column +HINT: Use ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION instead. \d gtest27 Table "public.gtest27" Column | Type | Collation | Nullable | Default @@ -628,6 +629,85 @@ ERROR: column "b" of relation "gtest27" is a generated column a | integer | | | b | numeric | | | generated always as ((a * 2)) stored +-- ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION +ALTER TABLE gtest27 ALTER COLUMN a DROP EXPRESSION; -- error +ERROR: column "a" of relation "gtest27" is not a stored generated column +ALTER TABLE gtest27 ALTER COLUMN a DROP EXPRESSION IF EXISTS; -- notice +NOTICE: column "a" of relation "gtest27" is not a stored generated column, skipping +ALTER TABLE gtest27 ALTER COLUMN b DROP EXPRESSION; +INSERT INTO gtest27 (a) VALUES (5); +INSERT INTO gtest27 (a, b) VALUES (6, 66); +SELECT * FROM gtest27; + a | b +---+---- + 3 | 6 + 4 | 8 + 5 | + 6 | 66 +(4 rows) + +\d gtest27 + Table "public.gtest27" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | numeric | | | + +-- check that dependencies between columns have also been removed +ALTER TABLE gtest27 DROP COLUMN a; -- should not drop b +\d gtest27 + Table "public.gtest27" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + b | numeric | | | + +-- with inheritance +CREATE TABLE gtest29 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest29_1 () INHERITS (gtest29); +ALTER TABLE gtest29 ALTER COLUMN b DROP EXPRESSION; +\d gtest29 + Table "public.gtest29" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Number of child tables: 1 (Use \d+ to list them.) + +\d gtest29_1 + Table "public.gtest29_1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Inherits: gtest29 + +DROP TABLE gtest29 CASCADE; +NOTICE: drop cascades to table gtest29_1 +CREATE TABLE gtest29 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest29_1 () INHERITS (gtest29); +ALTER TABLE ONLY gtest29 ALTER COLUMN b DROP EXPRESSION; +\d gtest29 + Table "public.gtest29" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Number of child tables: 1 (Use \d+ to list them.) + +\d gtest29_1 + Table "public.gtest29_1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+------------------------------------ + a | integer | | | + b | integer | | | generated always as (a * 2) stored +Inherits: gtest29 + -- 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 6a56ae260f..a981e616a1 100644 --- a/src/test/regress/sql/generated.sql +++ b/src/test/regress/sql/generated.sql @@ -332,6 +332,38 @@ CREATE TABLE gtest27 ( ALTER TABLE gtest27 ALTER COLUMN b DROP DEFAULT; -- error \d gtest27 +-- ALTER TABLE ... ALTER COLUMN ... DROP EXPRESSION +ALTER TABLE gtest27 ALTER COLUMN a DROP EXPRESSION; -- error +ALTER TABLE gtest27 ALTER COLUMN a DROP EXPRESSION IF EXISTS; -- notice +ALTER TABLE gtest27 ALTER COLUMN b DROP EXPRESSION; +INSERT INTO gtest27 (a) VALUES (5); +INSERT INTO gtest27 (a, b) VALUES (6, 66); +SELECT * FROM gtest27; +\d gtest27 + +-- check that dependencies between columns have also been removed +ALTER TABLE gtest27 DROP COLUMN a; -- should not drop b +\d gtest27 + +-- with inheritance +CREATE TABLE gtest29 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest29_1 () INHERITS (gtest29); +ALTER TABLE gtest29 ALTER COLUMN b DROP EXPRESSION; +\d gtest29 +\d gtest29_1 +DROP TABLE gtest29 CASCADE; +CREATE TABLE gtest29 ( + a int, + b int GENERATED ALWAYS AS (a * 2) STORED +); +CREATE TABLE gtest29_1 () INHERITS (gtest29); +ALTER TABLE ONLY gtest29 ALTER COLUMN b DROP EXPRESSION; +\d gtest29 +\d gtest29_1 + -- triggers CREATE TABLE gtest26 ( a int PRIMARY KEY, base-commit: 27a3b2ad836c9e7dd243bfebc760a9df9d6fd5a3 -- 2.24.1
