From f99d41331aff5c43c5a80878dca16b846719cbb1 Mon Sep 17 00:00:00 2001 From: Arpan Agrawal Date: Wed, 6 Nov 2024 17:09:13 +0530 Subject: [PATCH] [#24663] YSQL: Fix foreign key on unique index with different column orders Summary: For a foreign key constraint on the unique index of the referenced relation, the function YBCBuildYBTupleIdDescriptor() incorrectly assumes that the order of columns in foreign and unique key constraints would be the same. This leads to incorrect foreign key violation errors and data inconsistency issues when that is not the case (see the added test). Fix it by using the correct attribute number in the index relation. Jira: DB-13730 Test Plan: ./yb_build.sh --java-test org.yb.pgsql.TestPgForeignKey ./yb_build.sh --java-test org.yb.pgsql.TestPgForeignKeyBatching ./yb_build.sh --java-test org.yb.pgsql.TestPgRegressForeignKey Reviewers: dmitry Reviewed By: dmitry Subscribers: smishra, yql Differential Revision: https://phorge.dev.yugabyte.com/D39538 --- .../src/backend/access/yb_access/yb_scan.c | 19 ++----------------- .../src/backend/utils/adt/ri_triggers.c | 4 +++- .../src/backend/utils/misc/pg_yb_utils.c | 16 ++++++++++++++++ src/postgres/src/include/pg_yb_utils.h | 2 ++ .../test/regress/expected/yb_foreign_key.out | 13 +++++++++++++ .../src/test/regress/sql/yb_foreign_key.sql | 8 ++++++++ 6 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/postgres/src/backend/access/yb_access/yb_scan.c b/src/postgres/src/backend/access/yb_access/yb_scan.c index a6ae69c028f7..eb125e5975b6 100644 --- a/src/postgres/src/backend/access/yb_access/yb_scan.c +++ b/src/postgres/src/backend/access/yb_access/yb_scan.c @@ -990,21 +990,6 @@ YbGetLengthOfKey(ScanKey *key_ptr) return length_of_key; } -/* - * Given a table attribute number, get a corresponding index attribute number. - * Throw an error if it is not found. - */ -static AttrNumber -YbGetIndexAttnum(AttrNumber table_attno, Relation index) -{ - for (int i = 0; i < IndexRelationGetNumberOfAttributes(index); ++i) - { - if (table_attno == index->rd_index->indkey.values[i]) - return i + 1; - } - elog(ERROR, "column is not in index"); -} - /* * Add ordinary key to ybScan. */ @@ -2716,7 +2701,7 @@ YbDmlAppendTargetsAggregate(List *aggrefs, TupleDesc tupdesc, Relation index, * attribute number to an index-based one. */ if (index && xs_want_itup) - attno = YbGetIndexAttnum(attno, index); + attno = YbGetIndexAttnum(index, attno); Form_pg_attribute attr = TupleDescAttr(tupdesc, attno - 1); YBCPgTypeAttrs type_attrs = {attr->atttypmod}; @@ -3232,7 +3217,7 @@ SysScanDesc ybc_systable_begin_default_scan(Relation relation, * must be used for bindings. */ for (int i = 0; i < nkeys; ++i) - key[i].sk_attno = YbGetIndexAttnum(key[i].sk_attno, index); + key[i].sk_attno = YbGetIndexAttnum(index, key[i].sk_attno); } } diff --git a/src/postgres/src/backend/utils/adt/ri_triggers.c b/src/postgres/src/backend/utils/adt/ri_triggers.c index 222fbe2926cf..4f5fafc581ec 100644 --- a/src/postgres/src/backend/utils/adt/ri_triggers.c +++ b/src/postgres/src/backend/utils/adt/ri_triggers.c @@ -276,7 +276,9 @@ YBCBuildYBTupleIdDescriptor(const RI_ConstraintInfo *riinfo, TupleTableSlot *slo TupleDesc source_tupdesc = source_rel->rd_att; for (int i = 0; i < riinfo->nkeys; ++i, ++next_attr) { - next_attr->attr_num = using_index ? (i + 1) : riinfo->pk_attnums[i]; + next_attr->attr_num = + using_index ? YbGetIndexAttnum(source_rel, riinfo->pk_attnums[i]) : + riinfo->pk_attnums[i]; const int fk_attnum = riinfo->fk_attnums[i]; const Oid type_id = TupleDescAttr(slot->tts_tupleDescriptor, fk_attnum - 1)->atttypid; /* diff --git a/src/postgres/src/backend/utils/misc/pg_yb_utils.c b/src/postgres/src/backend/utils/misc/pg_yb_utils.c index b05b41020dc9..89e13b297a30 100644 --- a/src/postgres/src/backend/utils/misc/pg_yb_utils.c +++ b/src/postgres/src/backend/utils/misc/pg_yb_utils.c @@ -5678,3 +5678,19 @@ bool YbUseUnsafeTruncate(Relation rel) return IsYBRelation(rel) && (IsSystemRelation(rel) || !yb_enable_alter_table_rewrite); } + + +/* + * Given a table attribute number, get a corresponding index attribute number. + * Throw an error if it is not found. + */ +AttrNumber +YbGetIndexAttnum(Relation index, AttrNumber table_attno) +{ + for (int i = 0; i < IndexRelationGetNumberOfAttributes(index); ++i) + { + if (table_attno == index->rd_index->indkey.values[i]) + return i + 1; + } + elog(ERROR, "column is not in index"); +} diff --git a/src/postgres/src/include/pg_yb_utils.h b/src/postgres/src/include/pg_yb_utils.h index 66f4c2804e55..e2fc30a54c83 100644 --- a/src/postgres/src/include/pg_yb_utils.h +++ b/src/postgres/src/include/pg_yb_utils.h @@ -1253,6 +1253,8 @@ SortByDir YbGetIndexKeySortOrdering(Relation indexRel); bool YbUseUnsafeTruncate(Relation rel); +extern AttrNumber YbGetIndexAttnum(Relation index, AttrNumber table_attno); + extern bool yb_ysql_conn_mgr_superuser_existed; #endif /* PG_YB_UTILS_H */ diff --git a/src/postgres/src/test/regress/expected/yb_foreign_key.out b/src/postgres/src/test/regress/expected/yb_foreign_key.out index 41545b854d0c..48af6e9687bf 100644 --- a/src/postgres/src/test/regress/expected/yb_foreign_key.out +++ b/src/postgres/src/test/regress/expected/yb_foreign_key.out @@ -673,3 +673,16 @@ ERROR: insert or update on table "child_2" violates foreign key constraint "chi DETAIL: Key (v)=(1010002) is not present in table "parent". UPDATE child_1 SET v = v * 2 WHERE k < 5001; UPDATE child_2 SET v = (v - 1000000) * 2 + 1000000 WHERE k < 5001; +-- Test foreign key on unique index such that column orders in fk and unique constraints differ. +CREATE TABLE pk(id INT, name INT, add INT, PRIMARY KEY (id, name, add), UNIQUE (name, id)); +CREATE TABLE fk(id INT, name INT, FOREIGN KEY(id, name) REFERENCES pk(id, name)); +INSERT INTO pk VALUES (1, 500, 1000); +INSERT INTO fk VALUES (1, 500); +INSERT INTO fk VALUES (500, 1); -- should fail +ERROR: insert or update on table "fk" violates foreign key constraint "fk_id_name_fkey" +DETAIL: Key (id, name)=(500, 1) is not present in table "pk". +SELECT * from fk; + id | name +----+------ + 1 | 500 +(1 row) diff --git a/src/postgres/src/test/regress/sql/yb_foreign_key.sql b/src/postgres/src/test/regress/sql/yb_foreign_key.sql index 8e1c889d8771..73a62e96c170 100644 --- a/src/postgres/src/test/regress/sql/yb_foreign_key.sql +++ b/src/postgres/src/test/regress/sql/yb_foreign_key.sql @@ -419,3 +419,11 @@ UPDATE child_2 SET v = (v - 1000000) * 2 + 1000000 WHERE k <= 5001; -- should fa UPDATE child_1 SET v = v * 2 WHERE k < 5001; UPDATE child_2 SET v = (v - 1000000) * 2 + 1000000 WHERE k < 5001; + +-- Test foreign key on unique index such that column orders in fk and unique constraints differ. +CREATE TABLE pk(id INT, name INT, add INT, PRIMARY KEY (id, name, add), UNIQUE (name, id)); +CREATE TABLE fk(id INT, name INT, FOREIGN KEY(id, name) REFERENCES pk(id, name)); +INSERT INTO pk VALUES (1, 500, 1000); +INSERT INTO fk VALUES (1, 500); +INSERT INTO fk VALUES (500, 1); -- should fail +SELECT * from fk;