Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit47ea461

Browse files
committed
Fix handling of inherited check constraints in ALTER COLUMN TYPE (again).
The previous way of reconstructing check constraints was to do a separate"ALTER TABLE ONLY tab ADD CONSTRAINT" for each table in an inheritancehierarchy. However, that way has no hope of reconstructing the checkconstraints' own inheritance properties correctly, as pointed out inbug #13779 from Jan Dirk Zijlstra. What we should do instead is to doa regular "ALTER TABLE", allowing recursion, at the topmost table thathas a particular constraint, and then suppress the work queue entriesfor inherited instances of the constraint.Annoyingly, we'd tried to fix this behavior before, in commit5ed6546,but we failed to notice that it wasn't reconstructing the pg_constraintfield values correctly.As long as I'm touching pg_get_constraintdef_worker anyway, tweak it toalways schema-qualify the target table name; this seems like useful backupto the protections installed by commit5f17304.In HEAD/9.5, get rid of get_constraint_relation_oids, which is now unused.(I could alternatively have modified it to also return conislocal, but thatseemed like a pretty single-purpose API, so let's not pretend it has someother use.) It's unused in the back branches as well, but I left it inplace just in case some third-party code has decided to use it.In HEAD/9.5, also rename pg_get_constraintdef_string topg_get_constraintdef_command, as the previous name did nothing to explainwhat that entry point did differently from others (and its comment wasequally useless). Again, that change doesn't seem like material forback-patching.I did a bit of re-pgindenting in tablecmds.c in HEAD/9.5, as well.Otherwise, back-patch to all supported branches.
1 parent9892cc2 commit47ea461

File tree

4 files changed

+229
-41
lines changed

4 files changed

+229
-41
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3390,7 +3390,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
33903390
caseAT_ReAddConstraint:/* Re-add pre-existing check
33913391
* constraint */
33923392
ATExecAddConstraint(wqueue,tab,rel, (Constraint*)cmd->def,
3393-
false, true,lockmode);
3393+
true, true,lockmode);
33943394
break;
33953395
caseAT_AddIndexConstraint:/* ADD CONSTRAINT USING INDEX */
33963396
ATExecAddIndexConstraint(tab,rel, (IndexStmt*)cmd->def,lockmode);
@@ -5764,13 +5764,6 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
57645764
* AddRelationNewConstraints would normally assign different names to the
57655765
* child constraints. To fix that, we must capture the name assigned at
57665766
* the parent table and pass that down.
5767-
*
5768-
* When re-adding a previously existing constraint (during ALTER COLUMN TYPE),
5769-
* we don't need to recurse here, because recursion will be carried out at a
5770-
* higher level; the constraint name issue doesn't apply because the names
5771-
* have already been assigned and are just being re-used. We need a separate
5772-
* "is_readd" flag for that; just setting recurse=false would result in an
5773-
* error if there are child tables.
57745767
*/
57755768
staticvoid
57765769
ATAddCheckConstraint(List**wqueue,AlteredTableInfo*tab,Relationrel,
@@ -5798,7 +5791,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
57985791
*/
57995792
newcons=AddRelationNewConstraints(rel,NIL,
58005793
list_make1(copyObject(constr)),
5801-
recursing,/* allow_merge */
5794+
recursing |is_readd,/* allow_merge */
58025795
!recursing,/* is_local */
58035796
is_readd);/* is_internal */
58045797

@@ -5842,10 +5835,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
58425835

58435836
/*
58445837
* If adding a NO INHERIT constraint, no need to find our children.
5845-
* Likewise, in a re-add operation, we don't need to recurse (that will be
5846-
* handled at higher levels).
58475838
*/
5848-
if (constr->is_no_inherit||is_readd)
5839+
if (constr->is_no_inherit)
58495840
return;
58505841

58515842
/*
@@ -8204,10 +8195,30 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
82048195
def_item,tab->changedConstraintDefs)
82058196
{
82068197
OidoldId=lfirst_oid(oid_item);
8198+
HeapTupletup;
8199+
Form_pg_constraintcon;
82078200
Oidrelid;
82088201
Oidconfrelid;
8202+
boolconislocal;
8203+
8204+
tup=SearchSysCache1(CONSTROID,ObjectIdGetDatum(oldId));
8205+
if (!HeapTupleIsValid(tup))/* should not happen */
8206+
elog(ERROR,"cache lookup failed for constraint %u",oldId);
8207+
con= (Form_pg_constraint)GETSTRUCT(tup);
8208+
relid=con->conrelid;
8209+
confrelid=con->confrelid;
8210+
conislocal=con->conislocal;
8211+
ReleaseSysCache(tup);
8212+
8213+
/*
8214+
* If the constraint is inherited (only), we don't want to inject a
8215+
* new definition here; it'll get recreated when ATAddCheckConstraint
8216+
* recurses from adding the parent table's constraint. But we had to
8217+
* carry the info this far so that we can drop the constraint below.
8218+
*/
8219+
if (!conislocal)
8220+
continue;
82098221

8210-
get_constraint_relation_oids(oldId,&relid,&confrelid);
82118222
ATPostAlterTypeParse(oldId,relid,confrelid,
82128223
(char*)lfirst(def_item),
82138224
wqueue,lockmode,tab->rewrite);

‎src/backend/utils/adt/ruleutils.c

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ static Node *processIndirection(Node *node, deparse_context *context,
420420
staticvoidprintSubscripts(ArrayRef*aref,deparse_context*context);
421421
staticchar*get_relation_name(Oidrelid);
422422
staticchar*generate_relation_name(Oidrelid,List*namespaces);
423+
staticchar*generate_qualified_relation_name(Oidrelid);
423424
staticchar*generate_function_name(Oidfuncid,intnargs,
424425
List*argnames,Oid*argtypes,
425426
boolhas_variadic,bool*use_variadic_p);
@@ -1298,7 +1299,9 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
12981299
prettyFlags)));
12991300
}
13001301

1301-
/* Internal version that returns a palloc'd C string; no pretty-printing */
1302+
/*
1303+
* Internal version that returns a full ALTER TABLE ... ADD CONSTRAINT command
1304+
*/
13021305
char*
13031306
pg_get_constraintdef_string(OidconstraintId)
13041307
{
@@ -1347,10 +1350,16 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
13471350

13481351
initStringInfo(&buf);
13491352

1350-
if (fullCommand&&OidIsValid(conForm->conrelid))
1353+
if (fullCommand)
13511354
{
1352-
appendStringInfo(&buf,"ALTER TABLE ONLY %s ADD CONSTRAINT %s ",
1353-
generate_relation_name(conForm->conrelid,NIL),
1355+
/*
1356+
* Currently, callers want ALTER TABLE (without ONLY) for CHECK
1357+
* constraints, and other types of constraints don't inherit anyway so
1358+
* it doesn't matter whether we say ONLY or not. Someday we might
1359+
* need to let callers specify whether to put ONLY in the command.
1360+
*/
1361+
appendStringInfo(&buf,"ALTER TABLE %s ADD CONSTRAINT %s ",
1362+
generate_qualified_relation_name(conForm->conrelid),
13541363
quote_identifier(NameStr(conForm->conname)));
13551364
}
13561365

@@ -1874,28 +1883,9 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS)
18741883

18751884
if (OidIsValid(sequenceId))
18761885
{
1877-
HeapTupleclasstup;
1878-
Form_pg_classclasstuple;
1879-
char*nspname;
18801886
char*result;
18811887

1882-
/* Get the sequence's pg_class entry */
1883-
classtup=SearchSysCache1(RELOID,ObjectIdGetDatum(sequenceId));
1884-
if (!HeapTupleIsValid(classtup))
1885-
elog(ERROR,"cache lookup failed for relation %u",sequenceId);
1886-
classtuple= (Form_pg_class)GETSTRUCT(classtup);
1887-
1888-
/* Get the namespace */
1889-
nspname=get_namespace_name(classtuple->relnamespace);
1890-
if (!nspname)
1891-
elog(ERROR,"cache lookup failed for namespace %u",
1892-
classtuple->relnamespace);
1893-
1894-
/* And construct the result string */
1895-
result=quote_qualified_identifier(nspname,
1896-
NameStr(classtuple->relname));
1897-
1898-
ReleaseSysCache(classtup);
1888+
result=generate_qualified_relation_name(sequenceId);
18991889

19001890
PG_RETURN_TEXT_P(string_to_text(result));
19011891
}
@@ -9138,6 +9128,39 @@ generate_relation_name(Oid relid, List *namespaces)
91389128
returnresult;
91399129
}
91409130

9131+
/*
9132+
* generate_qualified_relation_name
9133+
*Compute the name to display for a relation specified by OID
9134+
*
9135+
* As above, but unconditionally schema-qualify the name.
9136+
*/
9137+
staticchar*
9138+
generate_qualified_relation_name(Oidrelid)
9139+
{
9140+
HeapTupletp;
9141+
Form_pg_classreltup;
9142+
char*relname;
9143+
char*nspname;
9144+
char*result;
9145+
9146+
tp=SearchSysCache1(RELOID,ObjectIdGetDatum(relid));
9147+
if (!HeapTupleIsValid(tp))
9148+
elog(ERROR,"cache lookup failed for relation %u",relid);
9149+
reltup= (Form_pg_class)GETSTRUCT(tp);
9150+
relname=NameStr(reltup->relname);
9151+
9152+
nspname=get_namespace_name(reltup->relnamespace);
9153+
if (!nspname)
9154+
elog(ERROR,"cache lookup failed for namespace %u",
9155+
reltup->relnamespace);
9156+
9157+
result=quote_qualified_identifier(nspname,relname);
9158+
9159+
ReleaseSysCache(tp);
9160+
9161+
returnresult;
9162+
}
9163+
91419164
/*
91429165
* generate_function_name
91439166
*Compute the name to display for a function specified by OID,

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

Lines changed: 128 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,16 +1805,125 @@ where oid = 'test_storage'::regclass;
18051805
t
18061806
(1 row)
18071807

1808-
-- ALTER TYPE with a check constraint and a child table (bugbefore Nov 2012)
1809-
CREATE TABLE test_inh_check (a float check (a > 10.2));
1808+
-- ALTERCOLUMNTYPE with a check constraint and a child table (bug#13779)
1809+
CREATE TABLE test_inh_check (a float check (a > 10.2), b float);
18101810
CREATE TABLE test_inh_check_child() INHERITS(test_inh_check);
1811+
\d test_inh_check
1812+
Table "public.test_inh_check"
1813+
Column | Type | Modifiers
1814+
--------+------------------+-----------
1815+
a | double precision |
1816+
b | double precision |
1817+
Check constraints:
1818+
"test_inh_check_a_check" CHECK (a > 10.2::double precision)
1819+
Number of child tables: 1 (Use \d+ to list them.)
1820+
1821+
\d test_inh_check_child
1822+
Table "public.test_inh_check_child"
1823+
Column | Type | Modifiers
1824+
--------+------------------+-----------
1825+
a | double precision |
1826+
b | double precision |
1827+
Check constraints:
1828+
"test_inh_check_a_check" CHECK (a > 10.2::double precision)
1829+
Inherits: test_inh_check
1830+
1831+
select relname, conname, coninhcount, conislocal, connoinherit
1832+
from pg_constraint c, pg_class r
1833+
where relname like 'test_inh_check%' and c.conrelid = r.oid
1834+
order by 1, 2;
1835+
relname | conname | coninhcount | conislocal | connoinherit
1836+
----------------------+------------------------+-------------+------------+--------------
1837+
test_inh_check | test_inh_check_a_check | 0 | t | f
1838+
test_inh_check_child | test_inh_check_a_check | 1 | f | f
1839+
(2 rows)
1840+
18111841
ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
1842+
\d test_inh_check
1843+
Table "public.test_inh_check"
1844+
Column | Type | Modifiers
1845+
--------+------------------+-----------
1846+
a | numeric |
1847+
b | double precision |
1848+
Check constraints:
1849+
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
1850+
Number of child tables: 1 (Use \d+ to list them.)
1851+
1852+
\d test_inh_check_child
1853+
Table "public.test_inh_check_child"
1854+
Column | Type | Modifiers
1855+
--------+------------------+-----------
1856+
a | numeric |
1857+
b | double precision |
1858+
Check constraints:
1859+
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
1860+
Inherits: test_inh_check
1861+
1862+
select relname, conname, coninhcount, conislocal, connoinherit
1863+
from pg_constraint c, pg_class r
1864+
where relname like 'test_inh_check%' and c.conrelid = r.oid
1865+
order by 1, 2;
1866+
relname | conname | coninhcount | conislocal | connoinherit
1867+
----------------------+------------------------+-------------+------------+--------------
1868+
test_inh_check | test_inh_check_a_check | 0 | t | f
1869+
test_inh_check_child | test_inh_check_a_check | 1 | f | f
1870+
(2 rows)
1871+
1872+
-- also try noinherit, local, and local+inherited cases
1873+
ALTER TABLE test_inh_check ADD CONSTRAINT bnoinherit CHECK (b > 100) NO INHERIT;
1874+
ALTER TABLE test_inh_check_child ADD CONSTRAINT blocal CHECK (b < 1000);
1875+
ALTER TABLE test_inh_check_child ADD CONSTRAINT bmerged CHECK (b > 1);
1876+
ALTER TABLE test_inh_check ADD CONSTRAINT bmerged CHECK (b > 1);
1877+
NOTICE: merging constraint "bmerged" with inherited definition
1878+
\d test_inh_check
1879+
Table "public.test_inh_check"
1880+
Column | Type | Modifiers
1881+
--------+------------------+-----------
1882+
a | numeric |
1883+
b | double precision |
1884+
Check constraints:
1885+
"bmerged" CHECK (b > 1::double precision)
1886+
"bnoinherit" CHECK (b > 100::double precision) NO INHERIT
1887+
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
1888+
Number of child tables: 1 (Use \d+ to list them.)
1889+
1890+
\d test_inh_check_child
1891+
Table "public.test_inh_check_child"
1892+
Column | Type | Modifiers
1893+
--------+------------------+-----------
1894+
a | numeric |
1895+
b | double precision |
1896+
Check constraints:
1897+
"blocal" CHECK (b < 1000::double precision)
1898+
"bmerged" CHECK (b > 1::double precision)
1899+
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
1900+
Inherits: test_inh_check
1901+
1902+
select relname, conname, coninhcount, conislocal, connoinherit
1903+
from pg_constraint c, pg_class r
1904+
where relname like 'test_inh_check%' and c.conrelid = r.oid
1905+
order by 1, 2;
1906+
relname | conname | coninhcount | conislocal | connoinherit
1907+
----------------------+------------------------+-------------+------------+--------------
1908+
test_inh_check | bmerged | 0 | t | f
1909+
test_inh_check | bnoinherit | 0 | t | t
1910+
test_inh_check | test_inh_check_a_check | 0 | t | f
1911+
test_inh_check_child | blocal | 0 | t | f
1912+
test_inh_check_child | bmerged | 1 | t | f
1913+
test_inh_check_child | test_inh_check_a_check | 1 | f | f
1914+
(6 rows)
1915+
1916+
ALTER TABLE test_inh_check ALTER COLUMN b TYPE numeric;
1917+
NOTICE: merging constraint "bmerged" with inherited definition
18121918
\d test_inh_check
18131919
Table "public.test_inh_check"
18141920
Column | Type | Modifiers
18151921
--------+---------+-----------
18161922
a | numeric |
1923+
b | numeric |
18171924
Check constraints:
1925+
"bmerged" CHECK (b::double precision > 1::double precision)
1926+
"bnoinherit" CHECK (b::double precision > 100::double precision) NO INHERIT
18181927
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
18191928
Number of child tables: 1 (Use \d+ to list them.)
18201929

@@ -1823,10 +1932,27 @@ Table "public.test_inh_check_child"
18231932
Column | Type | Modifiers
18241933
--------+---------+-----------
18251934
a | numeric |
1935+
b | numeric |
18261936
Check constraints:
1937+
"blocal" CHECK (b::double precision < 1000::double precision)
1938+
"bmerged" CHECK (b::double precision > 1::double precision)
18271939
"test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
18281940
Inherits: test_inh_check
18291941

1942+
select relname, conname, coninhcount, conislocal, connoinherit
1943+
from pg_constraint c, pg_class r
1944+
where relname like 'test_inh_check%' and c.conrelid = r.oid
1945+
order by 1, 2;
1946+
relname | conname | coninhcount | conislocal | connoinherit
1947+
----------------------+------------------------+-------------+------------+--------------
1948+
test_inh_check | bmerged | 0 | t | f
1949+
test_inh_check | bnoinherit | 0 | t | t
1950+
test_inh_check | test_inh_check_a_check | 0 | t | f
1951+
test_inh_check_child | blocal | 0 | t | f
1952+
test_inh_check_child | bmerged | 1 | t | f
1953+
test_inh_check_child | test_inh_check_a_check | 1 | f | f
1954+
(6 rows)
1955+
18301956
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
18311957
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
18321958
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,12 +1252,40 @@ select reltoastrelid <> 0 as has_toast_table
12521252
from pg_class
12531253
whereoid='test_storage'::regclass;
12541254

1255-
-- ALTER TYPE with a check constraint and a child table (bugbefore Nov 2012)
1256-
CREATETABLEtest_inh_check (a floatcheck (a>10.2));
1255+
-- ALTERCOLUMNTYPE with a check constraint and a child table (bug#13779)
1256+
CREATETABLEtest_inh_check (a floatcheck (a>10.2), b float);
12571257
CREATETABLEtest_inh_check_child() INHERITS(test_inh_check);
1258+
\d test_inh_check
1259+
\d test_inh_check_child
1260+
select relname, conname, coninhcount, conislocal, connoinherit
1261+
from pg_constraint c, pg_class r
1262+
where relnamelike'test_inh_check%'andc.conrelid=r.oid
1263+
order by1,2;
12581264
ALTERTABLE test_inh_check ALTER COLUMN a TYPEnumeric;
12591265
\d test_inh_check
12601266
\d test_inh_check_child
1267+
select relname, conname, coninhcount, conislocal, connoinherit
1268+
from pg_constraint c, pg_class r
1269+
where relnamelike'test_inh_check%'andc.conrelid=r.oid
1270+
order by1,2;
1271+
-- also try noinherit, local, and local+inherited cases
1272+
ALTERTABLE test_inh_check ADDCONSTRAINT bnoinheritCHECK (b>100) NO INHERIT;
1273+
ALTERTABLE test_inh_check_child ADDCONSTRAINT blocalCHECK (b<1000);
1274+
ALTERTABLE test_inh_check_child ADDCONSTRAINT bmergedCHECK (b>1);
1275+
ALTERTABLE test_inh_check ADDCONSTRAINT bmergedCHECK (b>1);
1276+
\d test_inh_check
1277+
\d test_inh_check_child
1278+
select relname, conname, coninhcount, conislocal, connoinherit
1279+
from pg_constraint c, pg_class r
1280+
where relnamelike'test_inh_check%'andc.conrelid=r.oid
1281+
order by1,2;
1282+
ALTERTABLE test_inh_check ALTER COLUMN b TYPEnumeric;
1283+
\d test_inh_check
1284+
\d test_inh_check_child
1285+
select relname, conname, coninhcount, conislocal, connoinherit
1286+
from pg_constraint c, pg_class r
1287+
where relnamelike'test_inh_check%'andc.conrelid=r.oid
1288+
order by1,2;
12611289

12621290
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
12631291
CREATETABLEcheck_fk_presence_1 (idintPRIMARY KEY, ttext);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp