From 01b73d3f27109e698724606764a8f7ec046952ed Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 23 Oct 2001 17:39:03 +0000 Subject: [PATCH] Fix foreign keys on system columns. --- src/backend/commands/command.c | 186 ++++------------------------ src/backend/parser/analyze.c | 45 +++---- src/backend/parser/parse_relation.c | 32 ++++- src/include/parser/parse_relation.h | 3 +- 4 files changed, 70 insertions(+), 196 deletions(-) diff --git a/src/backend/commands/command.c b/src/backend/commands/command.c index a81d097d8b..5297f7481d 100644 --- a/src/backend/commands/command.c +++ b/src/backend/commands/command.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.144 2001/10/12 00:07:14 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.145 2001/10/23 17:39:02 tgl Exp $ * * NOTES * The PerformAddAttribute() code, like most of the relation @@ -1347,9 +1347,7 @@ AlterTableAddConstraint(char *relationName, bool istemp = is_temp_rel_name(relationName); List *indexoidlist; List *indexoidscan; - Form_pg_attribute *rel_attrs; - int num_keys = 0; - int keys_matched = 0; + int num_keys; bool index_found = false; bool index_found_unique = false; bool index_found_primary = false; @@ -1394,15 +1392,9 @@ AlterTableAddConstraint(char *relationName, * constraint */ - rel_attrs = rel->rd_att->attrs; - - /* Retrieve the oids of all indices on the relation */ + /* Loop over all indices on the relation */ indexoidlist = RelationGetIndexList(rel); - index_found = false; - index_found_unique = false; - index_found_primary = false; - /* Loop over all indices on the relation */ foreach(indexoidscan, indexoidlist) { Oid indexoid = lfirsti(indexoidscan); @@ -1424,43 +1416,41 @@ AlterTableAddConstraint(char *relationName, * Make sure this index has the same number of * keys as the constraint -- It obviously won't match otherwise. */ - for (i = 0; i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0; i++); + for (i = 0; i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0; i++) + ; num_keys = length(constr->keys); - keys_matched = 0; if (i == num_keys) { /* Loop over each key in the constraint and check that there is a corresponding key in the index. */ + int keys_matched = 0; + i = 0; foreach(keyl, constr->keys) { Ident *key = lfirst(keyl); + int keyno = indexStruct->indkey[i]; /* Look at key[i] in the index and check that it is over the same column as key[i] in the constraint. This is to differentiate between (a,b) and (b,a) */ - if (i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0) - { - int keyno = indexStruct->indkey[i]; - - if (keyno > 0) - { - char *name = NameStr(rel_attrs[keyno - 1]->attname); - if (strcmp(name, key->name) == 0) keys_matched++; - } - } - else elog(ERROR, "ALTER TABLE/ADD CONSTRAINT: Key \"%u[%u]\" not found", indexoid, i); + if (namestrcmp(attnumAttName(rel, keyno), + key->name) == 0) + keys_matched++; + else + break; i++; } if (keys_matched == num_keys) { index_found = true; index_found_unique = indexStruct->indisunique; index_found_primary = indexStruct->indisprimary; - if (index_found_unique || index_found_primary) break; } } ReleaseSysCache(indexTuple); + if (index_found_unique || index_found_primary) + break; } freeList(indexoidlist); @@ -1504,19 +1494,7 @@ AlterTableAddConstraint(char *relationName, Trigger trig; List *list; int count; - List *indexoidlist, - *indexoidscan; - Form_pg_attribute *rel_attrs = NULL; - int i; - bool found = false; - - Oid fktypoid[INDEX_MAX_KEYS]; - Oid pktypoid[INDEX_MAX_KEYS]; - int attloc; - for (i=0; ipktable_name) && !is_temp_rel_name(relationName)) elog(ERROR, "ALTER TABLE / ADD CONSTRAINT: Unable to reference temporary table from permanent table constraint."); @@ -1530,140 +1508,21 @@ AlterTableAddConstraint(char *relationName, if (pkrel->rd_rel->relkind != RELKIND_RELATION) elog(ERROR, "referenced table \"%s\" not a relation", fkconstraint->pktable_name); + heap_close(pkrel, NoLock); /* + * First we check for limited correctness of the constraint. + * + * NOTE: we assume parser has already checked for existence + * of an appropriate unique index on the referenced relation, + * and that the column datatypes are comparable. + * * Scan through each tuple, calling the RI_FKey_Match_Ins * (insert trigger) as if that tuple had just been * inserted. If any of those fail, it should elog(ERROR) * and that's that. */ - /* - * First we check for limited correctness of the - * constraint - */ - - rel_attrs = pkrel->rd_att->attrs; - indexoidlist = RelationGetIndexList(pkrel); - - foreach(indexoidscan, indexoidlist) - { - Oid indexoid = lfirsti(indexoidscan); - HeapTuple indexTuple; - Form_pg_index indexStruct; - - indexTuple = SearchSysCache(INDEXRELID, - ObjectIdGetDatum(indexoid), - 0, 0, 0); - if (!HeapTupleIsValid(indexTuple)) - elog(ERROR, "transformFkeyGetPrimaryKey: index %u not found", - indexoid); - indexStruct = (Form_pg_index) GETSTRUCT(indexTuple); - - if (indexStruct->indisunique) - { - List *attrl; - - /* - * Make sure this index has the same number of - * keys -- It obviously won't match otherwise. - */ - for (i = 0; i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0; i++); - if (i != length(fkconstraint->pk_attrs)) - found = false; - else - { - attloc=0; - /* go through the fkconstraint->pk_attrs list */ - foreach(attrl, fkconstraint->pk_attrs) - { - Ident *attr = lfirst(attrl); - - found = false; - for (i = 0; i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0; i++) - { - int pkattno = indexStruct->indkey[i]; - - if (pkattno > 0) - { - char *name = NameStr(rel_attrs[pkattno - 1]->attname); - - if (strcmp(name, attr->name) == 0) - { - /* We get the type of this attribute here and - * store it so we can use it later for making - * sure the types are comparable. - */ - pktypoid[attloc++]=rel_attrs[pkattno-1]->atttypid; - found = true; - break; - } - } - } - if (!found) - break; - } - } - } - ReleaseSysCache(indexTuple); - if (found) - break; - } - - if (!found) - elog(ERROR, "UNIQUE constraint matching given keys for referenced table \"%s\" not found", - fkconstraint->pktable_name); - - freeList(indexoidlist); - heap_close(pkrel, NoLock); - - rel_attrs = rel->rd_att->attrs; - if (fkconstraint->fk_attrs != NIL) - { - List *fkattrs; - Ident *fkattr; - - found = false; - attloc = 0; - foreach(fkattrs, fkconstraint->fk_attrs) - { - int count; - - found = false; - fkattr = lfirst(fkattrs); - for (count = 0; count < rel->rd_att->natts; count++) - { - char *name = NameStr(rel->rd_att->attrs[count]->attname); - - if (strcmp(name, fkattr->name) == 0) - { - /* - * Here once again we get the types, this - * time for the fk table's attributes - */ - fktypoid[attloc++]=rel->rd_att->attrs[count]->atttypid; - found = true; - break; - } - } - if (!found) - break; - } - if (!found) - elog(ERROR, "columns referenced in foreign key constraint not found."); - } - - for (i=0; i < INDEX_MAX_KEYS && fktypoid[i] !=0; i++) { - /* - * fktypoid[i] is the foreign key table's i'th element's type oid - * pktypoid[i] is the primary key table's i'th element's type oid - * We let oper() do our work for us, including elog(ERROR) if the - * types can't compare with = - */ - Operator o=oper("=", fktypoid[i], pktypoid[i], false); - ReleaseSysCache(o); - } - trig.tgoid = 0; if (fkconstraint->constr_name) trig.tgname = fkconstraint->constr_name; @@ -1706,7 +1565,6 @@ AlterTableAddConstraint(char *relationName, trig.tgnargs = count - 1; scan = heap_beginscan(rel, false, SnapshotNow, 0, NULL); - AssertState(scan != NULL); while (HeapTupleIsValid(tuple = heap_getnext(scan, 0))) { diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 6c61ab9acb..6e89abefe8 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.202 2001/10/22 22:49:02 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.203 2001/10/23 17:39:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2428,7 +2428,6 @@ getSetColTypes(ParseState *pstate, Node *node) /* * transformUpdateStmt - * transforms an update statement - * */ static Query * transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt) @@ -2802,17 +2801,15 @@ static void transformFkeyCheckAttrs(FkConstraint *fkconstraint, Oid *pktypoid) { Relation pkrel; - Form_pg_attribute *pkrel_attrs; List *indexoidlist, *indexoidscan; int i; bool found = false; /* - * Open the referenced table and get the attributes list + * Open the referenced table */ pkrel = heap_openr(fkconstraint->pktable_name, AccessShareLock); - pkrel_attrs = pkrel->rd_att->attrs; /* * Get the list of index OIDs for the table from the relcache, and @@ -2827,6 +2824,7 @@ transformFkeyCheckAttrs(FkConstraint *fkconstraint, Oid *pktypoid) HeapTuple indexTuple; Form_pg_index indexStruct; + found = false; indexTuple = SearchSysCache(INDEXRELID, ObjectIdGetDatum(indexoid), 0, 0, 0); @@ -2837,16 +2835,14 @@ transformFkeyCheckAttrs(FkConstraint *fkconstraint, Oid *pktypoid) if (indexStruct->indisunique) { - List *attrl; - int attnum=0; - for (i = 0; i < INDEX_MAX_KEYS && indexStruct->indkey[i] != 0; i++) ; - if (i != length(fkconstraint->pk_attrs)) - found = false; - else + if (i == length(fkconstraint->pk_attrs)) { /* go through the fkconstraint->pk_attrs list */ + List *attrl; + int attnum = 0; + foreach(attrl, fkconstraint->pk_attrs) { Ident *attr = lfirst(attrl); @@ -2856,16 +2852,12 @@ transformFkeyCheckAttrs(FkConstraint *fkconstraint, Oid *pktypoid) { int pkattno = indexStruct->indkey[i]; - if (pkattno > 0) + if (namestrcmp(attnumAttName(pkrel, pkattno), + attr->name) == 0) { - char *name = NameStr(pkrel_attrs[pkattno - 1]->attname); - - if (strcmp(name, attr->name) == 0) - { - pktypoid[attnum++]=pkrel_attrs[pkattno-1]->atttypid; - found = true; - break; - } + pktypoid[attnum++] = attnumTypeId(pkrel, pkattno); + found = true; + break; } } if (!found) @@ -2896,7 +2888,6 @@ static void transformFkeyGetPrimaryKey(FkConstraint *fkconstraint, Oid *pktypoid) { Relation pkrel; - Form_pg_attribute *pkrel_attrs; List *indexoidlist, *indexoidscan; HeapTuple indexTuple = NULL; @@ -2905,10 +2896,9 @@ transformFkeyGetPrimaryKey(FkConstraint *fkconstraint, Oid *pktypoid) int attnum=0; /* - * Open the referenced table and get the attributes list + * Open the referenced table */ pkrel = heap_openr(fkconstraint->pktable_name, AccessShareLock); - pkrel_attrs = pkrel->rd_att->attrs; /* * Get the list of index OIDs for the table from the relcache, and @@ -2952,10 +2942,10 @@ transformFkeyGetPrimaryKey(FkConstraint *fkconstraint, Oid *pktypoid) int pkattno = indexStruct->indkey[i]; Ident *pkattr = makeNode(Ident); - pkattr->name = pstrdup(NameStr(pkrel_attrs[pkattno-1]->attname)); + pkattr->name = pstrdup(NameStr(*attnumAttName(pkrel, pkattno))); pkattr->indirection = NIL; pkattr->isRel = false; - pktypoid[attnum++] = pkrel_attrs[pkattno-1]->atttypid; + pktypoid[attnum++] = attnumTypeId(pkrel, pkattno); fkconstraint->pk_attrs = lappend(fkconstraint->pk_attrs, pkattr); } @@ -3023,6 +3013,7 @@ transformFkeyGetColType(CreateStmtContext *cxt, char *colname) List *cols; List *inher; Oid result; + Form_pg_attribute sysatt; /* First look for column among the newly-created columns */ foreach(cols, cxt->columns) @@ -3040,6 +3031,10 @@ transformFkeyGetColType(CreateStmtContext *cxt, char *colname) return result; } } + /* Perhaps it's a system column name */ + sysatt = SystemAttributeByName(colname, cxt->hasoids); + if (sysatt) + return sysatt->atttypid; /* Look for column among inherited columns (if CREATE TABLE case) */ foreach(inher, cxt->inhRelnames) { diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 5077d092da..10861bf273 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.57 2001/10/22 22:47:57 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.58 2001/10/23 17:39:02 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -938,7 +938,7 @@ attnameAttNum(Relation rd, char *a) int i; for (i = 0; i < rd->rd_rel->relnatts; i++) - if (!namestrcmp(&(rd->rd_att->attrs[i]->attname), a)) + if (namestrcmp(&(rd->rd_att->attrs[i]->attname), a) == 0) return i + 1; if ((i = specialAttNum(a)) != InvalidAttrNumber) @@ -974,6 +974,28 @@ specialAttNum(char *a) } +/* + * given attribute id, return name of that attribute + * + * This should only be used if the relation is already + * heap_open()'ed. Use the cache version get_atttype() + * for access to non-opened relations. + */ +Name +attnumAttName(Relation rd, int attid) +{ + if (attid <= 0) + { + Form_pg_attribute sysatt; + + sysatt = SystemAttributeDefinition(attid, rd->rd_rel->relhasoids); + return &sysatt->attname; + } + if (attid > rd->rd_att->natts) + elog(ERROR, "attnumAttName: invalid attribute number %d", attid); + return &rd->rd_att->attrs[attid - 1]->attname; +} + /* * given attribute id, return type of that attribute * @@ -991,10 +1013,8 @@ attnumTypeId(Relation rd, int attid) sysatt = SystemAttributeDefinition(attid, rd->rd_rel->relhasoids); return sysatt->atttypid; } - - /* - * -1 because attid is 1-based - */ + if (attid > rd->rd_att->natts) + elog(ERROR, "attnumTypeId: invalid attribute number %d", attid); return rd->rd_att->attrs[attid - 1]->atttypid; } diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index 82b3b0dd15..0cf4f40f19 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: parse_relation.h,v 1.24 2001/08/10 18:57:41 tgl Exp $ + * $Id: parse_relation.h,v 1.25 2001/10/23 17:39:03 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -45,6 +45,7 @@ extern List *expandRelAttrs(ParseState *pstate, RangeTblEntry *rte); extern List *expandJoinAttrs(ParseState *pstate, JoinExpr *join, int sublevels_up); extern int attnameAttNum(Relation rd, char *a); +extern Name attnumAttName(Relation rd, int attid); extern Oid attnumTypeId(Relation rd, int attid); #endif /* PARSE_RELATION_H */ -- 2.40.0