8000 Fix handling of inherited check constraints in ALTER COLUMN TYPE. · danielcode/postgres@329057f · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 329057f

Browse files
tglsfdctglsfdc
committed
Fix handling of inherited check constraints in ALTER COLUMN TYPE.
This case got broken in 8.4 by the addition of an error check that complains if ALTER TABLE ONLY is used on a table that has children. We do use ONLY for this situation, but it's okay because the necessary recursion occurs at a higher level. So we need to have a separate flag to suppress recursion without making the error check. Reported and patched by Pavan Deolasee, with some editorial adjustments by me. Back-patch to 8.4, since this is a regression of functionality that worked in earlier branches.
1 parent 2f5390a commit 329057f

File tree

4 files changed

+65
-11
lines changed

4 files changed

+65
-11
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -335,13 +335,15 @@ static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
335335
IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode);
336336
static void ATExecAddConstraint(List **wqueue,
337337
AlteredTableInfo *tab, Relation rel,
338-
Constraint *newConstraint, bool recurse, LOCKMODE lockmode);
338+
Constraint *newConstraint, bool recurse, bool is_readd,
339+
LOCKMODE lockmode);
339340
static void ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
340341
IndexStmt *stmt, LOCKMODE lockmode);
341342
static void ATAddCheckConstraint(List **wqueue,
342343
AlteredTableInfo *tab, Relation rel,
343344
Constraint *constr,
344-
bool recurse, bool recursing, LOCKMODE lockmode);
345+
bool recurse, bool recursing, bool is_readd,
346+
LOCKMODE lockmode);
345347
static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
346348
Constraint *fkconstraint, LOCKMODE lockmode);
347349
static void ATExecDropConstraint(Relation rel, const char *constrName,
@@ -2757,6 +2759,7 @@ AlterTableGetLockLevel(List *cmds)
27572759
case AT_ColumnDefault:
27582760
case AT_ProcessedConstraint: /* becomes AT_AddConstraint */
27592761
case AT_AddConstraintRecurse: /* becomes AT_AddConstraint */
2762+
case AT_ReAddConstraint: /* becomes AT_AddConstraint */
27602763
case AT_EnableTrig:
27612764
case AT_EnableAlwaysTrig:
27622765
case AT_EnableReplicaTrig:
@@ -3248,11 +3251,15 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
32483251
break;
32493252
case AT_AddConstraint: /* ADD CONSTRAINT */
32503253
ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
3251-
false, lockmode);
3254+
false, false, lockmode);
32523255
break;
32533256
case AT_AddConstraintRecurse: /* ADD CONSTRAINT with recursion */
32543257
ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
3255-
true, lockmode);
3258+
true, false, lockmode);
3259+
break;
3260+
case AT_ReAddConstraint: /* Re-add pre-existing check constraint */
3261+
ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
3262+
false, true, lockmode);
32563263
break;
32573264
case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */
32583265
ATExecAddIndexConstraint(tab, rel, (IndexStmt *) cmd->def, lockmode);
@@ -5499,7 +5506,8 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
54995506
*/
55005507
static void
55015508
ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
5502-
Constraint *newConstraint, bool recurse, LOCKMODE lockmode)
5509+
Constraint *newConstraint, bool recurse, bool is_readd,
5510+
LOCKMODE lockmode)
55035511
{
55045512
Assert(IsA(newConstraint, Constraint));
55055513

@@ -5512,7 +5520,8 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
55125520
{
55135521
case CONSTR_CHECK:
55145522
ATAddCheckConstraint(wqueue, tab, rel,
5515-
newConstraint, recurse, false, lockmode);
5523+
newConstraint, recurse, false, is_readd,
5524+
lockmode);
55165525
break;
55175526

55185527
case CONSTR_FOREIGN:
@@ -5564,11 +5573,18 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
55645573
* AddRelationNewConstraints would normally assign different names to the
55655574
* child constraints. To fix that, we must capture the name assigned at
55665575
* the parent table and pass that down.
5576+
*
5577+
* When re-adding a previously existing constraint (during ALTER COLUMN TYPE),
5578+
* we don't need to recurse here, because recursion will be carried out at a
5579+
* higher level; the constraint name issue doesn't apply because the names
5580+
* have already been assigned and are just being re-used. We need a separate
5581+
* "is_readd" flag for that; just setting recurse=false would result in an
5582+
* error if there are child tables.
55675583
*/
55685584
static void
55695585
ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
55705586
Constraint *constr, bool recurse, bool recursing,
5571-
LOCKMODE lockmode)
5587+
bool is_readd, LOCKMODE lockmode)
55725588
{
55735589
List *newcons;
55745590
ListCell *lcon;
@@ -5633,9 +5649,11 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
56335649
return;
56345650

56355651
/*
5636-
* Adding a NO INHERIT constraint? No need to find our children
5652+
* If adding a NO INHERIT constraint, no need to find our children.
5653+
* Likewise, in a re-add operation, we don't need to recurse (that will be
5654+
* handled at higher levels).
56375655
*/
5638-
if (constr->is_no_inherit)
5656+
if (constr->is_no_inherit || is_readd)
56395657
return;
56405658

56415659
/*
@@ -5670,7 +5688,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
56705688

56715689
/* Recurse to child */
56725690
ATAddCheckConstraint(wqueue, childtab, childrel,
5673-
constr, recurse, true, lockmode);
5691+
constr, recurse, true, is_readd, lockmode);
56745692

56755693
heap_close(childrel, NoLock);
56765694
}
@@ -7884,6 +7902,10 @@ ATPostAlterTypeParse(Oid oldId, char *cmd,
78847902
/*
78857903
* Attach each generated command to the proper place in the work queue.
78867904
* Note this could result in creation of entirely new work-queue entries.
7905+
*
7906+
* Also note that we have to tweak the command subtypes, because it turns
7907+
* out that re-creation of indexes and constraints has to act a bit
7908+
* differently from initial creation.
78877909
*/
78887910
foreach(list_item, querytree_list)
78897911
{
@@ -7941,6 +7963,7 @@ ATPostAlterTypeParse(Oid oldId, char *cmd,
79417963
if (con->contype == CONSTR_FOREIGN &&
79427964
!rewrite && !tab->rewrite)
79437965
TryReuseForeignKey(oldId, con);
7966+
cmd->subtype = AT_ReAddConstraint;
79447967
tab->subcmds[AT_PASS_OLD_CONSTR] =
79457968
lappend(tab->subcmds[AT_PASS_OLD_CONSTR], cmd);
79467969
break;

‎src/include/nodes/parsenodes.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1227,7 +1227,9 @@ typedef enum AlterTableType
12271227
AT_DropInherit, /* NO INHERIT parent */
12281228
AT_AddOf, /* OF <type_name> */
12291229
AT_DropOf, /* NOT OF */
1230-
AT_GenericOptions /* OPTIONS (...) */
1230+
AT_GenericOptions, /* OPTIONS (...) */
1231+
/* this will be in a more natural position in 9.3: */
1232+
AT_ReAddConstraint /* internal to commands/tablecmds.c */
12311233
} AlterTableType;
12321234

12331235
typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */

‎src/test/regress/expected/alter_table.out

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,6 +1804,28 @@ where oid = 'test_storage'::regclass;
18041804
t
18051805
(1 row)
18061806

1807+
-- ALTER TYPE with a check constraint and a child table (bug before Nov 2012)
1808+
CREATE TABLE test_inh_check (a float check (a > 10.2));
1809+
CREATE TABLE test_inh_check_child() INHERITS(test_inh_check);
1810+
ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
1811+
\d test_inh_check
1812+
Table "public.test_inh_check"
1813+
Column | Type | Modifiers
1814+
--------+---------+-----------
1815+
a | numeric |
1816+
Check constraints:
1817+
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
1818+
Number of child tables: 1 (Use \d+ to list them.)
1819+
1820+
\d test_inh_check_child
1821+
Table "public.test_inh_check_child"
1822+
Column | Type | Modifiers
1823+
--------+---------+-----------
1824+
a | numeric |
1825+
Check constraints:
1826+
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
1827+
Inherits: test_inh_check
1828+
18071829
--
18081830
-- lock levels
18091831
--

‎src/test/regress/sql/alter_table.sql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,13 @@ select reltoastrelid <> 0 as has_toast_table
12391239
from pg_class
12401240
where oid = 'test_storage'::regclass;
12411241

1242+
-- ALTER TYPE with a check constraint and a child table (bug before Nov 2012)
1243+
CREATE TABLE test_inh_check (a float check (a > 10.2));
1244+
CREATE TABLE test_inh_check_child() INHERITS(test_inh_check);
1245+
ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
1246+
\d test_inh_check
1247+
\d test_inh_check_child
1248+
12421249
--
12431250
-- lock levels
12441251
--

0 commit comments

Comments
 (0)
0