diff --git a/src/backend/utils/adt/datum.c b/src/backend/utils/adt/datum.c index af5944d395..81ea5a48e5 100644 --- a/src/backend/utils/adt/datum.c +++ b/src/backend/utils/adt/datum.c @@ -42,6 +42,8 @@ #include "postgres.h" +#include "access/tuptoaster.h" +#include "fmgr.h" #include "utils/datum.h" #include "utils/expandeddatum.h" @@ -251,6 +253,61 @@ datumIsEqual(Datum value1, Datum value2, bool typByVal, int typLen) return res; } +/*------------------------------------------------------------------------- + * datum_image_eq + * + * Compares two datums for identical contents, based on byte images. Return + * true if the two datums are equal, false otherwise. + *------------------------------------------------------------------------- + */ +bool +datum_image_eq(Datum value1, Datum value2, bool typByVal, int typLen) +{ + bool result = true; + + if (typLen == -1) + { + Size len1, + len2; + + len1 = toast_raw_datum_size(value1); + len2 = toast_raw_datum_size(value2); + /* No need to de-toast if lengths don't match. */ + if (len1 != len2) + result = false; + else + { + struct varlena *arg1val; + struct varlena *arg2val; + + arg1val = PG_DETOAST_DATUM_PACKED(value1); + arg2val = PG_DETOAST_DATUM_PACKED(value2); + + result = (memcmp(VARDATA_ANY(arg1val), + VARDATA_ANY(arg2val), + len1 - VARHDRSZ) == 0); + + /* Only free memory if it's a copy made here. */ + if ((Pointer) arg1val != (Pointer) value1) + pfree(arg1val); + if ((Pointer) arg2val != (Pointer) value2) + pfree(arg2val); + } + } + else if (typByVal) + { + result = (value1 == value2); + } + else + { + result = (memcmp(DatumGetPointer(value1), + DatumGetPointer(value2), + typLen) == 0); + } + + return result; +} + /*------------------------------------------------------------------------- * datumEstimateSpace * diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index d715709b7c..b4d0029877 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -42,6 +42,7 @@ #include "storage/bufmgr.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/datum.h" #include "utils/fmgroids.h" #include "utils/guc.h" #include "utils/inval.h" @@ -2402,18 +2403,11 @@ ri_KeysEqual(Relation rel, TupleTableSlot *oldslot, TupleTableSlot *newslot, const RI_ConstraintInfo *riinfo, bool rel_is_pk) { const int16 *attnums; - const Oid *eq_oprs; if (rel_is_pk) - { attnums = riinfo->pk_attnums; - eq_oprs = riinfo->pp_eq_oprs; - } else - { attnums = riinfo->fk_attnums; - eq_oprs = riinfo->ff_eq_oprs; - } /* XXX: could be worthwhile to fetch all necessary attrs at once */ for (int i = 0; i < riinfo->nkeys; i++) @@ -2436,12 +2430,32 @@ ri_KeysEqual(Relation rel, TupleTableSlot *oldslot, TupleTableSlot *newslot, if (isnull) return false; - /* - * Compare them with the appropriate equality operator. - */ - if (!ri_AttributesEqual(eq_oprs[i], RIAttType(rel, attnums[i]), - oldvalue, newvalue)) - return false; + if (rel_is_pk) + { + /* + * If we are looking at the PK table, then do a bytewise + * comparison. We must propagate PK changes if the value is + * changed to one that "looks" different but would compare as + * equal using the equality operator. This only makes a + * difference for ON UPDATE CASCADE, but for consistency we treat + * all changes to the PK the same. + */ + Form_pg_attribute att = TupleDescAttr(oldslot->tts_tupleDescriptor, attnums[i] - 1); + + if (!datum_image_eq(oldvalue, newvalue, att->attbyval, att->attlen)) + return false; + } + else + { + /* + * For the FK table, compare with the appropriate equality + * operator. Changes that compare equal will still satisfy the + * constraint after the update. + */ + if (!ri_AttributesEqual(riinfo->ff_eq_oprs[i], RIAttType(rel, attnums[i]), + oldvalue, newvalue)) + return false; + } } return true; diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c index 5bbf568610..aa7ec8735c 100644 --- a/src/backend/utils/adt/rowtypes.c +++ b/src/backend/utils/adt/rowtypes.c @@ -23,6 +23,7 @@ #include "libpq/pqformat.h" #include "miscadmin.h" #include "utils/builtins.h" +#include "utils/datum.h" #include "utils/lsyscache.h" #include "utils/typcache.h" @@ -1671,45 +1672,7 @@ record_image_eq(PG_FUNCTION_ARGS) } /* Compare the pair of elements */ - if (att1->attlen == -1) - { - Size len1, - len2; - - len1 = toast_raw_datum_size(values1[i1]); - len2 = toast_raw_datum_size(values2[i2]); - /* No need to de-toast if lengths don't match. */ - if (len1 != len2) - result = false; - else - { - struct varlena *arg1val; - struct varlena *arg2val; - - arg1val = PG_DETOAST_DATUM_PACKED(values1[i1]); - arg2val = PG_DETOAST_DATUM_PACKED(values2[i2]); - - result = (memcmp(VARDATA_ANY(arg1val), - VARDATA_ANY(arg2val), - len1 - VARHDRSZ) == 0); - - /* Only free memory if it's a copy made here. */ - if ((Pointer) arg1val != (Pointer) values1[i1]) - pfree(arg1val); - if ((Pointer) arg2val != (Pointer) values2[i2]) - pfree(arg2val); - } - } - else if (att1->attbyval) - { - result = (values1[i1] == values2[i2]); - } - else - { - result = (memcmp(DatumGetPointer(values1[i1]), - DatumGetPointer(values2[i2]), - att1->attlen) == 0); - } + result = datum_image_eq(values1[i1], values2[i2], att1->attbyval, att2->attlen); if (!result) break; } diff --git a/src/include/utils/datum.h b/src/include/utils/datum.h index 7b913578ab..4365bf06e6 100644 --- a/src/include/utils/datum.h +++ b/src/include/utils/datum.h @@ -46,6 +46,15 @@ extern Datum datumTransfer(Datum value, bool typByVal, int typLen); extern bool datumIsEqual(Datum value1, Datum value2, bool typByVal, int typLen); +/* + * datum_image_eq + * + * Compares two datums for identical contents, based on byte images. Return + * true if the two datums are equal, false otherwise. + */ +extern bool datum_image_eq(Datum value1, Datum value2, + bool typByVal, int typLen); + /* * Serialize and restore datums so that we can transfer them to parallel * workers. diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index f1a664e339..401514a3e0 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1494,6 +1494,40 @@ delete from pktable2 where f1 = 1; alter table fktable2 drop constraint fktable2_f1_fkey; ERROR: cannot ALTER TABLE "pktable2" because it has pending trigger events commit; +drop table pktable2, fktable2; +-- +-- Test keys that "look" different but compare as equal +-- +create table pktable2 (a float8, b float8, primary key (a, b)); +create table fktable2 (x float8, y float8, foreign key (x, y) references pktable2 (a, b) on update cascade); +insert into pktable2 values ('-0', '-0'); +insert into fktable2 values ('-0', '-0'); +select * from pktable2; + a | b +----+---- + -0 | -0 +(1 row) + +select * from fktable2; + x | y +----+---- + -0 | -0 +(1 row) + +update pktable2 set a = '0' where a = '-0'; +select * from pktable2; + a | b +---+---- + 0 | -0 +(1 row) + +-- should have updated fktable2.x +select * from fktable2; + x | y +---+---- + 0 | -0 +(1 row) + drop table pktable2, fktable2; -- -- Foreign keys and partitioned tables diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 4639fb4509..beeaf3277d 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1120,6 +1120,26 @@ commit; drop table pktable2, fktable2; +-- +-- Test keys that "look" different but compare as equal +-- +create table pktable2 (a float8, b float8, primary key (a, b)); +create table fktable2 (x float8, y float8, foreign key (x, y) references pktable2 (a, b) on update cascade); + +insert into pktable2 values ('-0', '-0'); +insert into fktable2 values ('-0', '-0'); + +select * from pktable2; +select * from fktable2; + +update pktable2 set a = '0' where a = '-0'; + +select * from pktable2; +-- should have updated fktable2.x +select * from fktable2; + +drop table pktable2, fktable2; + -- -- Foreign keys and partitioned tables