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

Commitfb466d7

Browse files
committed
Fully enforce uniqueness of constraint names.
It's been true for a long time that we expect names of table and domainconstraints to be unique among the constraints of that table or domain.However, the enforcement of that has been pretty haphazard, and it missedsome corner cases such as creating a CHECK constraint and then an indexconstraint of the same name (as per recent report from André Hänsel).Also, due to the lack of an actual unique index enforcing this, duplicatescould be created through race conditions.Moreover, the code that searches pg_constraint has been quite inconsistentabout how to handle duplicate names if one did occur: some places checkedand threw errors if there was more than one match, while others justprocessed the first match they came to.To fix, create a unique index on (conrelid, contypid, conname). Sinceeither conrelid or contypid is zero, this will separately enforceuniqueness of constraint names among constraints of any one table and anyone domain. (If we ever implement SQL assertions, and put them into thiscatalog, more thought might be needed. But it'd be at least as reasonableto put them into a new catalog; having overloaded this one catalog withtwo kinds of constraints was a mistake already IMO.) This index can replacethe existing non-unique index on conrelid, though we need to keep the oneon contypid for query performance reasons.Having done that, we can simplify the logic in various places that eithercoped with duplicates or neglected to, as well as potentially improvelookup performance when searching for a constraint by name.Also, as per our usual practice, install a preliminary check so that youget something more friendly than a unique-index violation report in thecase complained of by André. And teach ChooseIndexName to avoid choosingautogenerated names that would draw such a failure.While it's not possible to make such a change in the back branches,it doesn't seem quite too late to put this into v11, so do so.Discussion:https://postgr.es/m/0c1001d4428f$0942b430$1bc81c90$@webkr.de
1 parent2ce253c commitfb466d7

File tree

17 files changed

+494
-366
lines changed

17 files changed

+494
-366
lines changed

‎doc/src/sgml/ref/alter_index.sgml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable>
4848
<listitem>
4949
<para>
5050
The <literal>RENAME</literal> form changes the name of the index.
51+
If the index is associated with a table constraint (either
52+
<literal>UNIQUE</literal>, <literal>PRIMARY KEY</literal>,
53+
or <literal>EXCLUDE</literal>), the constraint is renamed as well.
5154
There is no effect on the stored data.
5255
</para>
5356
</listitem>

‎doc/src/sgml/ref/alter_table.sgml

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
474474
<term><literal>DROP CONSTRAINT [ IF EXISTS ]</literal></term>
475475
<listitem>
476476
<para>
477-
This form drops the specified constraint on a table.
477+
This form drops the specified constraint on a table, along with
478+
any index underlying the constraint.
478479
If <literal>IF EXISTS</literal> is specified and the constraint
479480
does not exist, no error is thrown. In this case a notice is issued instead.
480481
</para>
@@ -822,8 +823,10 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
822823
<listitem>
823824
<para>
824825
The <literal>RENAME</literal> forms change the name of a table
825-
(or an index, sequence, view, materialized view, or foreign table), the name
826-
of an individual column in a table, or the name of a constraint of the table.
826+
(or an index, sequence, view, materialized view, or foreign table), the
827+
name of an individual column in a table, or the name of a constraint of
828+
the table. When renaming a constraint that has an underlying index,
829+
the index is renamed as well.
827830
There is no effect on the stored data.
828831
</para>
829832
</listitem>
@@ -1270,10 +1273,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
12701273
<para>
12711274
If a table has any descendant tables, it is not permitted to add,
12721275
rename, or change the type of a column in the parent table without doing
1273-
same to the descendants. This ensures that the descendants always have
1274-
columns matching the parent. Similarly, a constraint cannot be renamed
1275-
in the parent without also renaming it in all descendants, so that
1276-
constraints also match between the parent and its descendants.
1276+
the same to the descendants. This ensures that the descendants always
1277+
have columns matching the parent. Similarly, a <literal>CHECK</literal>
1278+
constraint cannot be renamed in the parent without also renaming it in
1279+
all descendants, so that <literal>CHECK</literal> constraints also match
1280+
between the parent and its descendants. (That restriction does not apply
1281+
to index-based constraints, however.)
12771282
Also, because selecting from the parent also selects from its descendants,
12781283
a constraint on the parent cannot be marked valid unless it is also marked
12791284
valid for those descendants. In all of these cases, <command>ALTER TABLE
@@ -1481,35 +1486,35 @@ ALTER TABLE distributors DROP CONSTRAINT distributors_pkey,
14811486
</programlisting></para>
14821487

14831488
<para>
1484-
Attacha partition torangepartitioned table:
1489+
To attacha partition toa range-partitioned table:
14851490
<programlisting>
14861491
ALTER TABLE measurement
14871492
ATTACH PARTITION measurement_y2016m07 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
14881493
</programlisting></para>
14891494

14901495
<para>
1491-
Attacha partition tolistpartitioned table:
1496+
To attacha partition toa list-partitioned table:
14921497
<programlisting>
14931498
ALTER TABLE cities
14941499
ATTACH PARTITION cities_ab FOR VALUES IN ('a', 'b');
14951500
</programlisting></para>
14961501

14971502
<para>
1498-
Attach a defaultpartition to a partitioned table:
1503+
To attach apartition to ahash-partitioned table:
14991504
<programlisting>
1500-
ALTER TABLEcities
1501-
ATTACH PARTITIONcities_partdef DEFAULT;
1505+
ALTER TABLEorders
1506+
ATTACH PARTITIONorders_p4 FOR VALUES WITH (MODULUS 4, REMAINDER 3);
15021507
</programlisting></para>
15031508

15041509
<para>
1505-
Attach apartition tohash partitioned table:
1510+
To attach a defaultpartition toa partitioned table:
15061511
<programlisting>
1507-
ALTER TABLEorders
1508-
ATTACH PARTITIONorders_p4 FOR VALUES WITH (MODULUS 4, REMAINDER 3);
1512+
ALTER TABLEcities
1513+
ATTACH PARTITIONcities_partdef DEFAULT;
15091514
</programlisting></para>
15101515

15111516
<para>
1512-
Detacha partition from partitioned table:
1517+
To detacha partition from a partitioned table:
15131518
<programlisting>
15141519
ALTER TABLE measurement
15151520
DETACH PARTITION measurement_y2015m12;

‎doc/src/sgml/ref/create_table.sgml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1946,6 +1946,30 @@ CREATE TABLE cities_partdef
19461946
</para>
19471947
</refsect2>
19481948

1949+
<refsect2>
1950+
<title>Constraint Naming</title>
1951+
1952+
<para>
1953+
The SQL standard says that table and domain constraints must have names
1954+
that are unique across the schema containing the table or domain.
1955+
<productname>PostgreSQL</productname> is laxer: it only requires
1956+
constraint names to be unique across the constraints attached to a
1957+
particular table or domain. However, this extra freedom does not exist
1958+
for index-based constraints (<literal>UNIQUE</literal>,
1959+
<literal>PRIMARY KEY</literal>, and <literal>EXCLUDE</literal>
1960+
constraints), because the associated index is named the same as the
1961+
constraint, and index names must be unique across all relations within
1962+
the same schema.
1963+
</para>
1964+
1965+
<para>
1966+
Currently, <productname>PostgreSQL</productname> does not record names
1967+
for <literal>NOT NULL</literal> constraints at all, so they are not
1968+
subject to the uniqueness restriction. This might change in a future
1969+
release.
1970+
</para>
1971+
</refsect2>
1972+
19491973
<refsect2>
19501974
<title>Inheritance</title>
19511975

‎src/backend/catalog/heap.c

Lines changed: 95 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -2707,7 +2707,7 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
27072707
boolfound;
27082708
RelationconDesc;
27092709
SysScanDescconscan;
2710-
ScanKeyDataskey[2];
2710+
ScanKeyDataskey[3];
27112711
HeapTupletup;
27122712

27132713
/* Search for a pg_constraint entry with same name and relation */
@@ -2716,120 +2716,120 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
27162716
found= false;
27172717

27182718
ScanKeyInit(&skey[0],
2719+
Anum_pg_constraint_conrelid,
2720+
BTEqualStrategyNumber,F_OIDEQ,
2721+
ObjectIdGetDatum(RelationGetRelid(rel)));
2722+
ScanKeyInit(&skey[1],
2723+
Anum_pg_constraint_contypid,
2724+
BTEqualStrategyNumber,F_OIDEQ,
2725+
ObjectIdGetDatum(InvalidOid));
2726+
ScanKeyInit(&skey[2],
27192727
Anum_pg_constraint_conname,
27202728
BTEqualStrategyNumber,F_NAMEEQ,
27212729
CStringGetDatum(ccname));
27222730

2723-
ScanKeyInit(&skey[1],
2724-
Anum_pg_constraint_connamespace,
2725-
BTEqualStrategyNumber,F_OIDEQ,
2726-
ObjectIdGetDatum(RelationGetNamespace(rel)));
2727-
2728-
conscan=systable_beginscan(conDesc,ConstraintNameNspIndexId, true,
2729-
NULL,2,skey);
2731+
conscan=systable_beginscan(conDesc,ConstraintRelidTypidNameIndexId, true,
2732+
NULL,3,skey);
27302733

2731-
while (HeapTupleIsValid(tup=systable_getnext(conscan)))
2734+
/* There can be at most one matching row */
2735+
if (HeapTupleIsValid(tup=systable_getnext(conscan)))
27322736
{
27332737
Form_pg_constraintcon= (Form_pg_constraint)GETSTRUCT(tup);
27342738

2735-
if (con->conrelid==RelationGetRelid(rel))
2739+
/* Found it. Conflicts if not identical check constraint */
2740+
if (con->contype==CONSTRAINT_CHECK)
27362741
{
2737-
/* Found it. Conflicts if not identical check constraint */
2738-
if (con->contype==CONSTRAINT_CHECK)
2739-
{
2740-
Datumval;
2741-
boolisnull;
2742-
2743-
val=fastgetattr(tup,
2744-
Anum_pg_constraint_conbin,
2745-
conDesc->rd_att,&isnull);
2746-
if (isnull)
2747-
elog(ERROR,"null conbin for rel %s",
2748-
RelationGetRelationName(rel));
2749-
if (equal(expr,stringToNode(TextDatumGetCString(val))))
2750-
found= true;
2751-
}
2742+
Datumval;
2743+
boolisnull;
2744+
2745+
val=fastgetattr(tup,
2746+
Anum_pg_constraint_conbin,
2747+
conDesc->rd_att,&isnull);
2748+
if (isnull)
2749+
elog(ERROR,"null conbin for rel %s",
2750+
RelationGetRelationName(rel));
2751+
if (equal(expr,stringToNode(TextDatumGetCString(val))))
2752+
found= true;
2753+
}
27522754

2753-
/*
2754-
* If the existing constraint is purely inherited (no local
2755-
* definition) then interpret addition of a local constraint as a
2756-
* legal merge. This allows ALTER ADD CONSTRAINT on parent and
2757-
* child tables to be given in either order with same end state.
2758-
* However if the relation is a partition, all inherited
2759-
* constraints are always non-local, including those that were
2760-
* merged.
2761-
*/
2762-
if (is_local&& !con->conislocal&& !rel->rd_rel->relispartition)
2763-
allow_merge= true;
2755+
/*
2756+
* If the existing constraint is purely inherited (no local
2757+
* definition) then interpret addition of a local constraint as a
2758+
* legal merge. This allows ALTER ADD CONSTRAINT on parent and child
2759+
* tables to be given in either order with same end state. However if
2760+
* the relation is a partition, all inherited constraints are always
2761+
* non-local, including those that were merged.
2762+
*/
2763+
if (is_local&& !con->conislocal&& !rel->rd_rel->relispartition)
2764+
allow_merge= true;
27642765

2765-
if (!found|| !allow_merge)
2766-
ereport(ERROR,
2767-
(errcode(ERRCODE_DUPLICATE_OBJECT),
2768-
errmsg("constraint \"%s\" for relation \"%s\" already exists",
2769-
ccname,RelationGetRelationName(rel))));
2766+
if (!found|| !allow_merge)
2767+
ereport(ERROR,
2768+
(errcode(ERRCODE_DUPLICATE_OBJECT),
2769+
errmsg("constraint \"%s\" for relation \"%s\" already exists",
2770+
ccname,RelationGetRelationName(rel))));
27702771

2771-
/* If the child constraint is "no inherit" then cannot merge */
2772-
if (con->connoinherit)
2773-
ereport(ERROR,
2774-
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2775-
errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"",
2776-
ccname,RelationGetRelationName(rel))));
2772+
/* If the child constraint is "no inherit" then cannot merge */
2773+
if (con->connoinherit)
2774+
ereport(ERROR,
2775+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2776+
errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"",
2777+
ccname,RelationGetRelationName(rel))));
27772778

2778-
/*
2779-
* Must not change an existing inherited constraint to "no
2780-
*inherit"status. That's because inherited constraints should
2781-
* be able to propagate to lower-level children.
2782-
*/
2783-
if (con->coninhcount>0&&is_no_inherit)
2784-
ereport(ERROR,
2785-
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2786-
errmsg("constraint \"%s\" conflicts with inherited constraint on relation \"%s\"",
2787-
ccname,RelationGetRelationName(rel))));
2779+
/*
2780+
* Must not change an existing inherited constraint to "no inherit"
2781+
* status. That's because inherited constraints should be able to
2782+
* propagate to lower-level children.
2783+
*/
2784+
if (con->coninhcount>0&&is_no_inherit)
2785+
ereport(ERROR,
2786+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2787+
errmsg("constraint \"%s\" conflicts with inherited constraint on relation \"%s\"",
2788+
ccname,RelationGetRelationName(rel))));
27882789

2789-
/*
2790-
* If the child constraint is "not valid" then cannot merge with a
2791-
* valid parent constraint
2792-
*/
2793-
if (is_initially_valid&& !con->convalidated)
2794-
ereport(ERROR,
2795-
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2796-
errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"",
2797-
ccname,RelationGetRelationName(rel))));
2790+
/*
2791+
* If the child constraint is "not valid" then cannot merge with a
2792+
* valid parent constraint.
2793+
*/
2794+
if (is_initially_valid&& !con->convalidated)
2795+
ereport(ERROR,
2796+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2797+
errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"",
2798+
ccname,RelationGetRelationName(rel))));
27982799

2799-
/* OK to update the tuple */
2800-
ereport(NOTICE,
2801-
(errmsg("merging constraint \"%s\" with inherited definition",
2802-
ccname)));
2800+
/* OK to update the tuple */
2801+
ereport(NOTICE,
2802+
(errmsg("merging constraint \"%s\" with inherited definition",
2803+
ccname)));
28032804

2804-
tup=heap_copytuple(tup);
2805-
con= (Form_pg_constraint)GETSTRUCT(tup);
2805+
tup=heap_copytuple(tup);
2806+
con= (Form_pg_constraint)GETSTRUCT(tup);
28062807

2807-
/*
2808-
* In case of partitions, an inherited constraint must be
2809-
* inherited only once since it cannot have multiple parents and
2810-
* it is never considered local.
2811-
*/
2812-
if (rel->rd_rel->relispartition)
2813-
{
2814-
con->coninhcount=1;
2815-
con->conislocal= false;
2816-
}
2808+
/*
2809+
* In case of partitions, an inherited constraint must be inherited
2810+
* only once since it cannot have multiple parents and it is never
2811+
* considered local.
2812+
*/
2813+
if (rel->rd_rel->relispartition)
2814+
{
2815+
con->coninhcount=1;
2816+
con->conislocal= false;
2817+
}
2818+
else
2819+
{
2820+
if (is_local)
2821+
con->conislocal= true;
28172822
else
2818-
{
2819-
if (is_local)
2820-
con->conislocal= true;
2821-
else
2822-
con->coninhcount++;
2823-
}
2823+
con->coninhcount++;
2824+
}
28242825

2825-
if (is_no_inherit)
2826-
{
2827-
Assert(is_local);
2828-
con->connoinherit= true;
2829-
}
2830-
CatalogTupleUpdate(conDesc,&tup->t_self,tup);
2831-
break;
2826+
if (is_no_inherit)
2827+
{
2828+
Assert(is_local);
2829+
con->connoinherit= true;
28322830
}
2831+
2832+
CatalogTupleUpdate(conDesc,&tup->t_self,tup);
28332833
}
28342834

28352835
systable_endscan(conscan);

‎src/backend/catalog/index.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,12 @@ index_create(Relation heapRelation,
847847
if (shared_relation&&tableSpaceId!=GLOBALTABLESPACE_OID)
848848
elog(ERROR,"shared relations must be placed in pg_global tablespace");
849849

850+
/*
851+
* Check for duplicate name (both as to the index, and as to the
852+
* associated constraint if any). Such cases would fail on the relevant
853+
* catalogs' unique indexes anyway, but we prefer to give a friendlier
854+
* error message.
855+
*/
850856
if (get_relname_relid(indexRelationName,namespaceId))
851857
{
852858
if ((flags&INDEX_CREATE_IF_NOT_EXISTS)!=0)
@@ -865,6 +871,20 @@ index_create(Relation heapRelation,
865871
indexRelationName)));
866872
}
867873

874+
if ((flags&INDEX_CREATE_ADD_CONSTRAINT)!=0&&
875+
ConstraintNameIsUsed(CONSTRAINT_RELATION,heapRelationId,
876+
indexRelationName))
877+
{
878+
/*
879+
* INDEX_CREATE_IF_NOT_EXISTS does not apply here, since the
880+
* conflicting constraint is not an index.
881+
*/
882+
ereport(ERROR,
883+
(errcode(ERRCODE_DUPLICATE_OBJECT),
884+
errmsg("constraint \"%s\" for relation \"%s\" already exists",
885+
indexRelationName,RelationGetRelationName(heapRelation))));
886+
}
887+
868888
/*
869889
* construct tuple descriptor for index tuples
870890
*/

0 commit comments

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