Skip to content

Commit

Permalink
[#24663] YSQL: Fix foreign key on unique index with different column …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
arpang committed Nov 6, 2024
1 parent bd11a35 commit f99d413
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 18 deletions.
19 changes: 2 additions & 17 deletions src/postgres/src/backend/access/yb_access/yb_scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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};

Expand Down Expand Up @@ -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);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/postgres/src/backend/utils/adt/ri_triggers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/*
Expand Down
16 changes: 16 additions & 0 deletions src/postgres/src/backend/utils/misc/pg_yb_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
2 changes: 2 additions & 0 deletions src/postgres/src/include/pg_yb_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
13 changes: 13 additions & 0 deletions src/postgres/src/test/regress/expected/yb_foreign_key.out
Original file line number Diff line number Diff line change
Expand Up @@ -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)
8 changes: 8 additions & 0 deletions src/postgres/src/test/regress/sql/yb_foreign_key.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;

0 comments on commit f99d413

Please sign in to comment.