Skip to content

Commit d4f6265

Browse files
bmomjianbmomjian
committed
This is mostly the same as an earlier patch I
didn't hear anything about, but which would have broken with the function manager changes anyway. Well, this patch checks that a unique constraint of some form (unique or pk) is on the referenced columns of an FK constraint and that the columns in the referencing table exist at creation time. The former is to move closer to SQL compatibility and the latter is in answer to a bug report. I also added a basic check of this functionality to the alter table and foreign key regression tests. Stephan Szabo sszabo@bigpanda.com
1 parent c51041f commit d4f6265

File tree

6 files changed

+272
-7
lines changed

6 files changed

+272
-7
lines changed

‎src/backend/commands/command.c‎

Lines changed: 90 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.96 2000/08/25 18:05:54 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.97 2000/08/29 04:20:43 momjian Exp $
1212
*
1313
* NOTES
1414
* The PerformAddAttribute() code, like most of the relation
@@ -45,9 +45,10 @@
4545
#include "commands/view.h"
4646
#include "utils/temprel.h"
4747
#include "executor/spi_priv.h"
48+
#include "catalog/pg_index.h"
49+
#include "utils/relcache.h"
4850

4951
#ifdef _DROP_COLUMN_HACK__
50-
#include "catalog/pg_index.h"
5152
#include "parser/parse.h"
5253
#endif /* _DROP_COLUMN_HACK__ */
5354
#include "access/genam.h"
@@ -1241,12 +1242,18 @@ AlterTableAddConstraint(char *relationName,
12411242
case T_FkConstraint:
12421243
{
12431244
FkConstraint *fkconstraint = (FkConstraint *) newConstraint;
1244-
Relation rel;
1245+
Relation rel, pkrel;
12451246
HeapScanDesc scan;
12461247
HeapTuple tuple;
12471248
Trigger trig;
12481249
List *list;
12491250
int count;
1251+
List *indexoidlist,
1252+
*indexoidscan;
1253+
Form_pg_index indexStruct = NULL;
1254+
Form_pg_attribute *rel_attrs = NULL;
1255+
int i;
1256+
int found=0;
12501257

12511258
if (get_temp_rel_by_username(fkconstraint->pktable_name)!=NULL &&
12521259
get_temp_rel_by_username(relationName)==NULL) {
@@ -1273,8 +1280,10 @@ AlterTableAddConstraint(char *relationName,
12731280
* doesn't delete rows out from under us.
12741281
*/
12751282

1276-
rel = heap_openr(fkconstraint->pktable_name, AccessExclusiveLock);
1277-
heap_close(rel, NoLock);
1283+
pkrel = heap_openr(fkconstraint->pktable_name, AccessExclusiveLock);
1284+
if (pkrel == NULL)
1285+
elog(ERROR, "referenced table \"%s\" not found",
1286+
fkconstraint->pktable_name);
12781287

12791288
/*
12801289
* Grab an exclusive lock on the fk table, and then scan
@@ -1284,6 +1293,82 @@ AlterTableAddConstraint(char *relationName,
12841293
* and that's that.
12851294
*/
12861295
rel = heap_openr(relationName, AccessExclusiveLock);
1296+
if (rel == NULL)
1297+
elog(ERROR, "table \"%s\" not found",
1298+
relationName);
1299+
1300+
/* First we check for limited correctness of the constraint */
1301+
1302+
rel_attrs = pkrel->rd_att->attrs;
1303+
indexoidlist = RelationGetIndexList(pkrel);
1304+
1305+
foreach(indexoidscan, indexoidlist)
1306+
{
1307+
Oid indexoid = lfirsti(indexoidscan);
1308+
HeapTuple indexTuple;
1309+
List *attrl;
1310+
indexTuple = SearchSysCacheTuple(INDEXRELID,
1311+
ObjectIdGetDatum(indexoid),
1312+
0, 0, 0);
1313+
if (!HeapTupleIsValid(indexTuple))
1314+
elog(ERROR, "transformFkeyGetPrimaryKey: index %u not found",
1315+
indexoid);
1316+
indexStruct = (Form_pg_index) GETSTRUCT(indexTuple);
1317+
1318+
if (indexStruct->indisunique) {
1319+
/* go through the fkconstraint->pk_attrs list */
1320+
foreach(attrl, fkconstraint->pk_attrs) {
1321+
Ident *attr=lfirst(attrl);
1322+
found=0;
1323+
for (i = 0; i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0; i++)
1324+
{
1325+
int pkattno = indexStruct->indkey[i];
1326+
if (pkattno>0) {
1327+
char *name = NameStr(rel_attrs[pkattno-1]->attname);
1328+
if (strcmp(name, attr->name)==0) {
1329+
found=1;
1330+
break;
1331+
}
1332+
}
1333+
}
1334+
if (!found)
1335+
break;
1336+
}
1337+
}
1338+
if (found)
1339+
break;
1340+
indexStruct = NULL;
1341+
}
1342+
if (!found)
1343+
elog(ERROR, "UNIQUE constraint matching given keys for referenced table \"%s\" not found",
1344+
fkconstraint->pktable_name);
1345+
1346+
freeList(indexoidlist);
1347+
heap_close(pkrel, NoLock);
1348+
1349+
rel_attrs = rel->rd_att->attrs;
1350+
if (fkconstraint->fk_attrs!=NIL) {
1351+
int found=0;
1352+
List *fkattrs;
1353+
Ident *fkattr;
1354+
foreach(fkattrs, fkconstraint->fk_attrs) {
1355+
int count=0;
1356+
found=0;
1357+
fkattr=lfirst(fkattrs);
1358+
for (; count < rel->rd_att->natts; count++) {
1359+
char *name = NameStr(rel->rd_att->attrs[count]->attname);
1360+
if (strcmp(name, fkattr->name)==0) {
1361+
found=1;
1362+
break;
1363+
}
1364+
}
1365+
if (!found)
1366+
break;
1367+
}
1368+
if (!found)
1369+
elog(ERROR, "columns referenced in foreign key constraint not found.");
1370+
}
1371+
12871372
trig.tgoid = 0;
12881373
if (fkconstraint->constr_name)
12891374
trig.tgname = fkconstraint->constr_name;

‎src/backend/parser/analyze.c‎

Lines changed: 149 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2000, PostgreSQL, Inc
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Id: analyze.c,v 1.155 2000/08/22 12:59:04 ishii Exp $
9+
* $Id: analyze.c,v 1.156 2000/08/29 04:20:44 momjian Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -52,6 +52,7 @@ static void transformForUpdate(Query *qry, List *forUpdate);
5252
static void transformFkeyGetPrimaryKey(FkConstraint *fkconstraint);
5353
static void transformConstraintAttrs(List *constraintList);
5454
static void transformColumnType(ParseState *pstate, ColumnDef *column);
55+
static void transformFkeyCheckAttrs(FkConstraint *fkconstraint);
5556

5657
/* kluge to return extra info from transformCreateStmt() */
5758
static List *extras_before;
@@ -1062,6 +1063,33 @@ transformCreateStmt(ParseState *pstate, CreateStmt *stmt)
10621063
if (fkconstraint->constr_name == NULL)
10631064
fkconstraint->constr_name = "<unnamed>";
10641065

1066+
/*
1067+
* Check to see if the attributes mentioned by the constraint
1068+
* actually exist on this table.
1069+
*/
1070+
if (fkconstraint->fk_attrs!=NIL) {
1071+
int found=0;
1072+
List *cols;
1073+
List *fkattrs;
1074+
Ident *fkattr;
1075+
ColumnDef *col;
1076+
foreach(fkattrs, fkconstraint->fk_attrs) {
1077+
found=0;
1078+
fkattr=lfirst(fkattrs);
1079+
foreach(cols, columns) {
1080+
col=lfirst(cols);
1081+
if (strcmp(col->colname, fkattr->name)==0) {
1082+
found=1;
1083+
break;
1084+
}
1085+
}
1086+
if (!found)
1087+
break;
1088+
}
1089+
if (!found)
1090+
elog(ERROR, "columns referenced in foreign key constraint not found.");
1091+
}
1092+
10651093
/*
10661094
* If the attribute list for the referenced table was omitted,
10671095
* lookup for the definition of the primary key. If the
@@ -1096,7 +1124,43 @@ transformCreateStmt(ParseState *pstate, CreateStmt *stmt)
10961124
fkconstraint->pktable_name);
10971125
}
10981126
}
1099-
1127+
else {
1128+
if (strcmp(fkconstraint->pktable_name, stmt->relname)!=0)
1129+
transformFkeyCheckAttrs(fkconstraint);
1130+
else {
1131+
/* Get a unique/pk constraint from above */
1132+
List *index;
1133+
int found=0;
1134+
foreach(index, ilist)
1135+
{
1136+
IndexStmt *ind=lfirst(index);
1137+
IndexElem *indparm;
1138+
List *indparms;
1139+
List *pkattrs;
1140+
Ident *pkattr;
1141+
if (ind->unique) {
1142+
foreach(pkattrs, fkconstraint->pk_attrs) {
1143+
found=0;
1144+
pkattr=lfirst(pkattrs);
1145+
foreach(indparms, ind->indexParams) {
1146+
indparm=lfirst(indparms);
1147+
if (strcmp(indparm->name, pkattr->name)==0) {
1148+
found=1;
1149+
break;
1150+
}
1151+
}
1152+
if (!found)
1153+
break;
1154+
}
1155+
}
1156+
if (found)
1157+
break;
1158+
}
1159+
if (!found)
1160+
elog(ERROR, "UNIQUE constraint matching given keys for referenced table \"%s\" not found",
1161+
fkconstraint->pktable_name);
1162+
}
1163+
}
11001164
/*
11011165
* Build a CREATE CONSTRAINT TRIGGER statement for the CHECK
11021166
* action.
@@ -2029,6 +2093,89 @@ transformForUpdate(Query *qry, List *forUpdate)
20292093
}
20302094

20312095

2096+
/*
2097+
* transformFkeyCheckAttrs -
2098+
*
2099+
* Try to make sure that the attributes of a referenced table
2100+
* belong to a unique (or primary key) constraint.
2101+
*
2102+
*/
2103+
static void
2104+
transformFkeyCheckAttrs(FkConstraint *fkconstraint)
2105+
{
2106+
Relation pkrel;
2107+
Form_pg_attribute *pkrel_attrs;
2108+
List *indexoidlist,
2109+
*indexoidscan;
2110+
Form_pg_index indexStruct = NULL;
2111+
int i;
2112+
int found=0;
2113+
2114+
/* ----------
2115+
* Open the referenced table and get the attributes list
2116+
* ----------
2117+
*/
2118+
pkrel = heap_openr(fkconstraint->pktable_name, AccessShareLock);
2119+
if (pkrel == NULL)
2120+
elog(ERROR, "referenced table \"%s\" not found",
2121+
fkconstraint->pktable_name);
2122+
pkrel_attrs = pkrel->rd_att->attrs;
2123+
2124+
/* ----------
2125+
* Get the list of index OIDs for the table from the relcache,
2126+
* and look up each one in the pg_index syscache for each unique
2127+
* one, and then compare the attributes we were given to those
2128+
* defined.
2129+
* ----------
2130+
*/
2131+
indexoidlist = RelationGetIndexList(pkrel);
2132+
2133+
foreach(indexoidscan, indexoidlist)
2134+
{
2135+
Oid indexoid = lfirsti(indexoidscan);
2136+
HeapTuple indexTuple;
2137+
List *attrl;
2138+
indexTuple = SearchSysCacheTuple(INDEXRELID,
2139+
ObjectIdGetDatum(indexoid),
2140+
0, 0, 0);
2141+
if (!HeapTupleIsValid(indexTuple))
2142+
elog(ERROR, "transformFkeyGetPrimaryKey: index %u not found",
2143+
indexoid);
2144+
indexStruct = (Form_pg_index) GETSTRUCT(indexTuple);
2145+
2146+
if (indexStruct->indisunique) {
2147+
/* go through the fkconstraint->pk_attrs list */
2148+
foreach(attrl, fkconstraint->pk_attrs) {
2149+
Ident *attr=lfirst(attrl);
2150+
found=0;
2151+
for (i = 0; i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0; i++)
2152+
{
2153+
int pkattno = indexStruct->indkey[i];
2154+
if (pkattno>0) {
2155+
char *name = NameStr(pkrel_attrs[pkattno - 1]->attname);
2156+
if (strcmp(name, attr->name)==0) {
2157+
found=1;
2158+
break;
2159+
}
2160+
}
2161+
}
2162+
if (!found)
2163+
break;
2164+
}
2165+
}
2166+
if (found)
2167+
break;
2168+
indexStruct = NULL;
2169+
}
2170+
if (!found)
2171+
elog(ERROR, "UNIQUE constraint matching given keys for referenced table \"%s\" not found",
2172+
fkconstraint->pktable_name);
2173+
2174+
freeList(indexoidlist);
2175+
heap_close(pkrel, AccessShareLock);
2176+
}
2177+
2178+
20322179
/*
20332180
* transformFkeyGetPrimaryKey -
20342181
*

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,14 @@ INSERT INTO tmp2 values (4);
282282
INSERT INTO tmp3 values (1,10);
283283
INSERT INTO tmp3 values (1,20);
284284
INSERT INTO tmp3 values (5,50);
285+
-- Try (and fail) to add constraint due to invalid source columns
286+
ALTER TABLE tmp3 add constraint tmpconstr foreign key(c) references tmp2 match full;
287+
NOTICE: ALTER TABLE ... ADD CONSTRAINT will create implicit trigger(s) for FOREIGN KEY check(s)
288+
ERROR: columns referenced in foreign key constraint not found.
289+
-- Try (and fail) to add constraint due to invalide destination columns explicitly given
290+
ALTER TABLE tmp3 add constraint tmpconstr foreign key(a) references tmp2(b) match full;
291+
NOTICE: ALTER TABLE ... ADD CONSTRAINT will create implicit trigger(s) for FOREIGN KEY check(s)
292+
ERROR: UNIQUE constraint matching given keys for referenced table "tmp2" not found
285293
-- Try (and fail) to add constraint due to invalid data
286294
ALTER TABLE tmp3 add constraint tmpconstr foreign key (a) references tmp2 match full;
287295
NOTICE: ALTER TABLE ... ADD CONSTRAINT will create implicit trigger(s) for FOREIGN KEY check(s)

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,3 +690,16 @@ DROP TABLE FKTABLE;
690690
NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "pktable"
691691
NOTICE: DROP TABLE implicitly drops referential integrity trigger from table "pktable"
692692
DROP TABLE PKTABLE;
693+
CREATE TABLE PKTABLE (ptest1 int PRIMARY KEY);
694+
NOTICE: CREATE TABLE/PRIMARY KEY will create implicit index 'pktable_pkey' for table 'pktable'
695+
CREATE TABLE FKTABLE_FAIL1 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest2) REFERENCES PKTABLE);
696+
NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
697+
ERROR: columns referenced in foreign key constraint not found.
698+
CREATE TABLE FKTABLE_FAIL2 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest1) REFERENCES PKTABLE(ptest2));
699+
NOTICE: CREATE TABLE will create implicit trigger(s) for FOREIGN KEY check(s)
700+
ERROR: UNIQUE constraint matching given keys for referenced table "pktable" not found
701+
DROP TABLE FKTABLE_FAIL1;
702+
ERROR: Relation 'fktable_fail1' does not exist
703+
DROP TABLE FKTABLE_FAIL2;
704+
ERROR: Relation 'fktable_fail2' does not exist
705+
DROP TABLE PKTABLE;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,12 @@ INSERT INTO tmp3 values (1,10);
180180
INSERT INTO tmp3 values (1,20);
181181
INSERT INTO tmp3 values (5,50);
182182

183+
-- Try (and fail) to add constraint due to invalid source columns
184+
ALTER TABLE tmp3 add constraint tmpconstr foreign key(c) references tmp2 match full;
185+
186+
-- Try (and fail) to add constraint due to invalide destination columns explicitly given
187+
ALTER TABLE tmp3 add constraint tmpconstr foreign key(a) references tmp2(b) match full;
188+
183189
-- Try (and fail) to add constraint due to invalid data
184190
ALTER TABLE tmp3 add constraint tmpconstr foreign key (a) references tmp2 match full;
185191

‎src/test/regress/sql/foreign_key.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,4 +411,10 @@ SELECT * from FKTABLE;
411411
DROP TABLE FKTABLE;
412412
DROP TABLE PKTABLE;
413413

414+
CREATE TABLE PKTABLE (ptest1 int PRIMARY KEY);
415+
CREATE TABLE FKTABLE_FAIL1 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest2) REFERENCES PKTABLE);
416+
CREATE TABLE FKTABLE_FAIL2 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest1) REFERENCES PKTABLE(ptest2));
414417

418+
DROP TABLE FKTABLE_FAIL1;
419+
DROP TABLE FKTABLE_FAIL2;
420+
DROP TABLE PKTABLE;

0 commit comments

Comments
 (0)