From bfb993b1bbd137b5ea0a4f14bddd57ac46dac494 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 31 Mar 2023 11:18:49 -0400 Subject: [PATCH] Reject system columns as elements of foreign keys. Up through v11 it was sensible to use the "oid" system column as a foreign key column, but since that was removed there's no visible usefulness in making any of the remaining system columns a foreign key. Moreover, since the TupleTableSlot rewrites in v12, such cases actively fail because of implicit assumptions that only user columns appear in foreign keys. The lack of complaints about that seems like good evidence that no one is trying to do it. Hence, rather than trying to repair those assumptions (of which there are at least two, maybe more), let's just forbid the case up front. Per this patch, a system column in either the referenced or referencing side of a foreign key will draw this error; however, putting one in the referenced side would have failed later anyway, since we don't allow unique indexes to be made on system columns. Per bug #17877 from Alexander Lakhin. Back-patch to v12; the case still appears to work in v11, so we shouldn't break it there. Discussion: https://postgr.es/m/17877-4bcc658e33df6de1@postgresql.org --- src/backend/commands/tablecmds.c | 15 +++++++++++++-- src/test/regress/expected/foreign_key.out | 11 ++++++----- src/test/regress/sql/foreign_key.sql | 7 ++++--- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 5c596499d1..03148358c5 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10407,6 +10407,11 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName, * transformColumnNameList - transform list of column names * * Lookup each name and return its attnum and type OID + * + * Note: the name of this function suggests that it's general-purpose, + * but actually it's only used to look up names appearing in foreign-key + * clauses. The error messages would need work to use it in other cases, + * and perhaps the validity checks as well. */ static int transformColumnNameList(Oid relId, List *colList, @@ -10420,6 +10425,7 @@ transformColumnNameList(Oid relId, List *colList, { char *attname = strVal(lfirst(l)); HeapTuple atttuple; + Form_pg_attribute attform; atttuple = SearchSysCacheAttName(relId, attname); if (!HeapTupleIsValid(atttuple)) @@ -10427,13 +10433,18 @@ transformColumnNameList(Oid relId, List *colList, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" referenced in foreign key constraint does not exist", attname))); + attform = (Form_pg_attribute) GETSTRUCT(atttuple); + if (attform->attnum < 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("system columns cannot be used in foreign keys"))); if (attnum >= INDEX_MAX_KEYS) ereport(ERROR, (errcode(ERRCODE_TOO_MANY_COLUMNS), errmsg("cannot have more than %d keys in a foreign key", INDEX_MAX_KEYS))); - attnums[attnum] = ((Form_pg_attribute) GETSTRUCT(atttuple))->attnum; - atttypids[attnum] = ((Form_pg_attribute) GETSTRUCT(atttuple))->atttypid; + attnums[attnum] = attform->attnum; + atttypids[attnum] = attform->atttypid; ReleaseSysCache(atttuple); attnum++; } diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index e235e90d27..71e85ff731 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -757,15 +757,16 @@ SELECT * from FKTABLE; DROP TABLE FKTABLE; DROP TABLE PKTABLE; -CREATE TABLE PKTABLE (ptest1 int PRIMARY KEY); +-- Test some invalid FK definitions +CREATE TABLE PKTABLE (ptest1 int PRIMARY KEY, someoid oid); CREATE TABLE FKTABLE_FAIL1 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest2) REFERENCES PKTABLE); ERROR: column "ftest2" referenced in foreign key constraint does not exist CREATE TABLE FKTABLE_FAIL2 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest1) REFERENCES PKTABLE(ptest2)); ERROR: column "ptest2" referenced in foreign key constraint does not exist -DROP TABLE FKTABLE_FAIL1; -ERROR: table "fktable_fail1" does not exist -DROP TABLE FKTABLE_FAIL2; -ERROR: table "fktable_fail2" does not exist +CREATE TABLE FKTABLE_FAIL3 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (tableoid) REFERENCES PKTABLE(someoid)); +ERROR: system columns cannot be used in foreign keys +CREATE TABLE FKTABLE_FAIL4 ( ftest1 oid, CONSTRAINT fkfail1 FOREIGN KEY (ftest1) REFERENCES PKTABLE(tableoid)); +ERROR: system columns cannot be used in foreign keys DROP TABLE PKTABLE; -- Test for referencing column number smaller than referenced constraint CREATE TABLE PKTABLE (ptest1 int, ptest2 int, UNIQUE(ptest1, ptest2)); diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index c188ade85f..057e131d88 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -463,12 +463,13 @@ SELECT * from FKTABLE; DROP TABLE FKTABLE; DROP TABLE PKTABLE; -CREATE TABLE PKTABLE (ptest1 int PRIMARY KEY); +-- Test some invalid FK definitions +CREATE TABLE PKTABLE (ptest1 int PRIMARY KEY, someoid oid); CREATE TABLE FKTABLE_FAIL1 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest2) REFERENCES PKTABLE); CREATE TABLE FKTABLE_FAIL2 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (ftest1) REFERENCES PKTABLE(ptest2)); +CREATE TABLE FKTABLE_FAIL3 ( ftest1 int, CONSTRAINT fkfail1 FOREIGN KEY (tableoid) REFERENCES PKTABLE(someoid)); +CREATE TABLE FKTABLE_FAIL4 ( ftest1 oid, CONSTRAINT fkfail1 FOREIGN KEY (ftest1) REFERENCES PKTABLE(tableoid)); -DROP TABLE FKTABLE_FAIL1; -DROP TABLE FKTABLE_FAIL2; DROP TABLE PKTABLE; -- Test for referencing column number smaller than referenced constraint