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

Commit64349f1

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 parentae81d4f commit64349f1

File tree

4 files changed

+232
-44
lines changed

4 files changed

+232
-44
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3298,7 +3298,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
32983298
caseAT_ReAddConstraint:/* Re-add pre-existing check
32993299
* constraint */
33003300
ATExecAddConstraint(wqueue,tab,rel, (Constraint*)cmd->def,
3301-
false, true,lockmode);
3301+
true, true,lockmode);
33023302
break;
33033303
caseAT_AddIndexConstraint:/* ADD CONSTRAINT USING INDEX */
33043304
ATExecAddIndexConstraint(tab,rel, (IndexStmt*)cmd->def,lockmode);
@@ -5649,13 +5649,6 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
56495649
* AddRelationNewConstraints would normally assign different names to the
56505650
* child constraints. To fix that, we must capture the name assigned at
56515651
* the parent table and pass that down.
5652-
*
5653-
* When re-adding a previously existing constraint (during ALTER COLUMN TYPE),
5654-
* we don't need to recurse here, because recursion will be carried out at a
5655-
* higher level; the constraint name issue doesn't apply because the names
5656-
* have already been assigned and are just being re-used. We need a separate
5657-
* "is_readd" flag for that; just setting recurse=false would result in an
5658-
* error if there are child tables.
56595652
*/
56605653
staticvoid
56615654
ATAddCheckConstraint(List**wqueue,AlteredTableInfo*tab,Relationrel,
@@ -5683,7 +5676,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
56835676
*/
56845677
newcons=AddRelationNewConstraints(rel,NIL,
56855678
list_make1(copyObject(constr)),
5686-
recursing,/* allow_merge */
5679+
recursing |is_readd,/* allow_merge */
56875680
!recursing,/* is_local */
56885681
is_readd);/* is_internal */
56895682

@@ -5727,10 +5720,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
57275720

57285721
/*
57295722
* If adding a NO INHERIT constraint, no need to find our children.
5730-
* Likewise, in a re-add operation, we don't need to recurse (that will be
5731-
* handled at higher levels).
57325723
*/
5733-
if (constr->is_no_inherit||is_readd)
5724+
if (constr->is_no_inherit)
57345725
return;
57355726

57365727
/*
@@ -7937,11 +7928,31 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
79377928
forboth(oid_item,tab->changedConstraintOids,
79387929
def_item,tab->changedConstraintDefs)
79397930
{
7940-
OidoldId=lfirst_oid(oid_item);
7941-
Oidrelid;
7942-
Oidconfrelid;
7931+
OidoldId=lfirst_oid(oid_item);
7932+
HeapTupletup;
7933+
Form_pg_constraintcon;
7934+
Oidrelid;
7935+
Oidconfrelid;
7936+
boolconislocal;
7937+
7938+
tup=SearchSysCache1(CONSTROID,ObjectIdGetDatum(oldId));
7939+
if (!HeapTupleIsValid(tup))/* should not happen */
7940+
elog(ERROR,"cache lookup failed for constraint %u",oldId);
7941+
con= (Form_pg_constraint)GETSTRUCT(tup);
7942+
relid=con->conrelid;
7943+
confrelid=con->confrelid;
7944+
conislocal=con->conislocal;
7945+
ReleaseSysCache(tup);
7946+
7947+
/*
7948+
* If the constraint is inherited (only), we don't want to inject a
7949+
* new definition here; it'll get recreated when ATAddCheckConstraint
7950+
* recurses from adding the parent table's constraint. But we had to
7951+
* carry the info this far so that we can drop the constraint below.
7952+
*/
7953+
if (!conislocal)
7954+
continue;
79437955

7944-
get_constraint_relation_oids(oldId,&relid,&confrelid);
79457956
ATPostAlterTypeParse(oldId,relid,confrelid,
79467957
(char*)lfirst(def_item),
79477958
wqueue,lockmode,tab->rewrite);

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

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ static Node *processIndirection(Node *node, deparse_context *context,
418418
staticvoidprintSubscripts(ArrayRef*aref,deparse_context*context);
419419
staticchar*get_relation_name(Oidrelid);
420420
staticchar*generate_relation_name(Oidrelid,List*namespaces);
421+
staticchar*generate_qualified_relation_name(Oidrelid);
421422
staticchar*generate_function_name(Oidfuncid,intnargs,
422423
List*argnames,Oid*argtypes,
423424
boolwas_variadic,bool*use_variadic_p);
@@ -1296,7 +1297,9 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
12961297
prettyFlags)));
12971298
}
12981299

1299-
/* Internal version that returns a palloc'd C string; no pretty-printing */
1300+
/*
1301+
* Internal version that returns a full ALTER TABLE ... ADD CONSTRAINT command
1302+
*/
13001303
char*
13011304
pg_get_constraintdef_string(OidconstraintId)
13021305
{
@@ -1318,10 +1321,16 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
13181321

13191322
initStringInfo(&buf);
13201323

1321-
if (fullCommand&&OidIsValid(conForm->conrelid))
1324+
if (fullCommand)
13221325
{
1323-
appendStringInfo(&buf,"ALTER TABLE ONLY %s ADD CONSTRAINT %s ",
1324-
generate_relation_name(conForm->conrelid,NIL),
1326+
/*
1327+
* Currently, callers want ALTER TABLE (without ONLY) for CHECK
1328+
* constraints, and other types of constraints don't inherit anyway so
1329+
* it doesn't matter whether we say ONLY or not. Someday we might
1330+
* need to let callers specify whether to put ONLY in the command.
1331+
*/
1332+
appendStringInfo(&buf,"ALTER TABLE %s ADD CONSTRAINT %s ",
1333+
generate_qualified_relation_name(conForm->conrelid),
13251334
quote_identifier(NameStr(conForm->conname)));
13261335
}
13271336

@@ -1844,28 +1853,9 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS)
18441853

18451854
if (OidIsValid(sequenceId))
18461855
{
1847-
HeapTupleclasstup;
1848-
Form_pg_classclasstuple;
1849-
char*nspname;
18501856
char*result;
18511857

1852-
/* Get the sequence's pg_class entry */
1853-
classtup=SearchSysCache1(RELOID,ObjectIdGetDatum(sequenceId));
1854-
if (!HeapTupleIsValid(classtup))
1855-
elog(ERROR,"cache lookup failed for relation %u",sequenceId);
1856-
classtuple= (Form_pg_class)GETSTRUCT(classtup);
1857-
1858-
/* Get the namespace */
1859-
nspname=get_namespace_name(classtuple->relnamespace);
1860-
if (!nspname)
1861-
elog(ERROR,"cache lookup failed for namespace %u",
1862-
classtuple->relnamespace);
1863-
1864-
/* And construct the result string */
1865-
result=quote_qualified_identifier(nspname,
1866-
NameStr(classtuple->relname));
1867-
1868-
ReleaseSysCache(classtup);
1858+
result=generate_qualified_relation_name(sequenceId);
18691859

18701860
PG_RETURN_TEXT_P(string_to_text(result));
18711861
}
@@ -8862,6 +8852,39 @@ generate_relation_name(Oid relid, List *namespaces)
88628852
returnresult;
88638853
}
88648854

8855+
/*
8856+
* generate_qualified_relation_name
8857+
*Compute the name to display for a relation specified by OID
8858+
*
8859+
* As above, but unconditionally schema-qualify the name.
8860+
*/
8861+
staticchar*
8862+
generate_qualified_relation_name(Oidrelid)
8863+
{
8864+
HeapTupletp;
8865+
Form_pg_classreltup;
8866+
char*relname;
8867+
char*nspname;
8868+
char*result;
8869+
8870+
tp=SearchSysCache1(RELOID,ObjectIdGetDatum(relid));
8871+
if (!HeapTupleIsValid(tp))
8872+
elog(ERROR,"cache lookup failed for relation %u",relid);
8873+
reltup= (Form_pg_class)GETSTRUCT(tp);
8874+
relname=NameStr(reltup->relname);
8875+
8876+
nspname=get_namespace_name(reltup->relnamespace);
8877+
if (!nspname)
8878+
elog(ERROR,"cache lookup failed for namespace %u",
8879+
reltup->relnamespace);
8880+
8881+
result=quote_qualified_identifier(nspname,relname);
8882+
8883+
ReleaseSysCache(tp);
8884+
8885+
returnresult;
8886+
}
8887+
88658888
/*
88668889
* generate_function_name
88678890
*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