On Sun, Feb 12, 2012 at 7:36 AM, Vik Reykja <vikrey...@gmail.com> wrote:
> I decided to take a crack at the todo item created from the following post: > http://archives.postgresql.org/pgsql-performance/2005-10/msg00458.php > > The attached patch makes the desired changes in both code and function > naming. > > It seemed quite easy to do but wasn't marked as easy on the todo, so I'm > wondering if I've missed something. All regression tests pass. > > The patch was not getting applied. Was seeing below message: postgresql$ git apply /Downloads/unchanged.patch error: src/backend/utils/adt/ri_triggers.c: already exists in working directory Have come up with attached patch which hopefully should not have missed any of your changes. Please verify the changes. Regards, Chetan PS: would like the patch name to be something meaningful. -- EnterpriseDB Corporation The Enterprise PostgreSQL Company Website: www.enterprisedb.com EnterpriseDB Blog : http://blogs.enterprisedb.com Follow us on Twitter : http://www.twitter.com/enterprisedb
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 03a974a..09bacb7 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -205,11 +205,11 @@ static void ri_BuildQueryKeyFull(RI_QueryKey *key, static void ri_BuildQueryKeyPkCheck(RI_QueryKey *key, const RI_ConstraintInfo *riinfo, int32 constr_queryno); -static bool ri_KeysEqual(Relation rel, HeapTuple oldtup, HeapTuple newtup, +static bool ri_KeysUnchanged(Relation rel, HeapTuple oldtup, HeapTuple newtup, const RI_ConstraintInfo *riinfo, bool rel_is_pk); -static bool ri_AllKeysUnequal(Relation rel, HeapTuple oldtup, HeapTuple newtup, +static bool ri_AllKeysChanged(Relation rel, HeapTuple oldtup, HeapTuple newtup, const RI_ConstraintInfo *riinfo, bool rel_is_pk); -static bool ri_OneKeyEqual(Relation rel, int column, +static bool ri_OneKeyUnchanged(Relation rel, int column, HeapTuple oldtup, HeapTuple newtup, const RI_ConstraintInfo *riinfo, bool rel_is_pk); static bool ri_AttributesEqual(Oid eq_opr, Oid typeid, @@ -932,9 +932,9 @@ RI_FKey_noaction_upd(PG_FUNCTION_ARGS) } /* - * No need to check anything if old and new keys are equal + * No need to check anything if old and new keys are unchanged */ - if (ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true)) + if (ri_KeysUnchanged(pk_rel, old_row, new_row, &riinfo, true)) { heap_close(fk_rel, RowShareLock); return PointerGetDatum(NULL); @@ -1281,9 +1281,9 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS) } /* - * No need to do anything if old and new keys are equal + * No need to do anything if old and new keys are unchanged */ - if (ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true)) + if (ri_KeysUnchanged(pk_rel, old_row, new_row, &riinfo, true)) { heap_close(fk_rel, RowExclusiveLock); return PointerGetDatum(NULL); @@ -1646,9 +1646,9 @@ RI_FKey_restrict_upd(PG_FUNCTION_ARGS) } /* - * No need to check anything if old and new keys are equal + * No need to check anything if old and new keys are unchanged */ - if (ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true)) + if (ri_KeysUnchanged(pk_rel, old_row, new_row, &riinfo, true)) { heap_close(fk_rel, RowShareLock); return PointerGetDatum(NULL); @@ -1993,9 +1993,9 @@ RI_FKey_setnull_upd(PG_FUNCTION_ARGS) } /* - * No need to do anything if old and new keys are equal + * No need to do anything if old and new keys are unchanged */ - if (ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true)) + if (ri_KeysUnchanged(pk_rel, old_row, new_row, &riinfo, true)) { heap_close(fk_rel, RowExclusiveLock); return PointerGetDatum(NULL); @@ -2012,13 +2012,10 @@ RI_FKey_setnull_upd(PG_FUNCTION_ARGS) * our cached plan, unless the update happens to change all * columns in the key. Fortunately, for the most common case of a * single-column foreign key, this will be true. - * - * In case you're wondering, the inequality check works because we - * know that the old key value has no NULLs (see above). */ use_cached_query = (riinfo.confmatchtype == FKCONSTR_MATCH_FULL) || - ri_AllKeysUnequal(pk_rel, old_row, new_row, + ri_AllKeysChanged(pk_rel, old_row, new_row, &riinfo, true); /* @@ -2064,7 +2061,7 @@ RI_FKey_setnull_upd(PG_FUNCTION_ARGS) * to changed columns in pk_rel's key */ if (riinfo.confmatchtype == FKCONSTR_MATCH_FULL || - !ri_OneKeyEqual(pk_rel, i, old_row, new_row, + !ri_OneKeyUnchanged(pk_rel, i, old_row, new_row, &riinfo, true)) { appendStringInfo(&querybuf, @@ -2389,9 +2386,9 @@ RI_FKey_setdefault_upd(PG_FUNCTION_ARGS) } /* - * No need to do anything if old and new keys are equal + * No need to do anything if old and new keys are unchanged */ - if (ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true)) + if (ri_KeysUnchanged(pk_rel, old_row, new_row, &riinfo, true)) { heap_close(fk_rel, RowExclusiveLock); return PointerGetDatum(NULL); @@ -2443,7 +2440,7 @@ RI_FKey_setdefault_upd(PG_FUNCTION_ARGS) * to changed columns in pk_rel's key */ if (riinfo.confmatchtype == FKCONSTR_MATCH_FULL || - !ri_OneKeyEqual(pk_rel, i, old_row, new_row, + !ri_OneKeyUnchanged(pk_rel, i, old_row, new_row, &riinfo, true)) { appendStringInfo(&querybuf, @@ -2540,8 +2537,8 @@ RI_FKey_keyequal_upd_pk(Trigger *trigger, Relation pk_rel, { case FKCONSTR_MATCH_UNSPECIFIED: case FKCONSTR_MATCH_FULL: - /* Return true if keys are equal */ - return ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true); + /* Return true if keys are unchanged */ + return ri_KeysUnchanged(pk_rel, old_row, new_row, &riinfo, true); /* Handle MATCH PARTIAL set null delete. */ case FKCONSTR_MATCH_PARTIAL: @@ -2585,8 +2582,8 @@ RI_FKey_keyequal_upd_fk(Trigger *trigger, Relation fk_rel, { case FKCONSTR_MATCH_UNSPECIFIED: case FKCONSTR_MATCH_FULL: - /* Return true if keys are equal */ - return ri_KeysEqual(fk_rel, old_row, new_row, &riinfo, false); + /* Return true if keys are unchanged */ + return ri_KeysUnchanged(fk_rel, old_row, new_row, &riinfo, false); /* Handle MATCH PARTIAL set null delete. */ case FKCONSTR_MATCH_PARTIAL: @@ -3763,13 +3760,13 @@ ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan) /* ---------- - * ri_KeysEqual - + * ri_KeysUnchanged - * - * Check if all key values in OLD and NEW are equal. + * Check if all key values in OLD and NEW are unchanged. * ---------- */ static bool -ri_KeysEqual(Relation rel, HeapTuple oldtup, HeapTuple newtup, +ri_KeysUnchanged(Relation rel, HeapTuple oldtup, HeapTuple newtup, const RI_ConstraintInfo *riinfo, bool rel_is_pk) { TupleDesc tupdesc = RelationGetDescr(rel); @@ -3792,21 +3789,21 @@ ri_KeysEqual(Relation rel, HeapTuple oldtup, HeapTuple newtup, { Datum oldvalue; Datum newvalue; - bool isnull; + bool isoldnull; + bool isnewnull; /* - * Get one attribute's oldvalue. If it is NULL - they're not equal. - */ - oldvalue = SPI_getbinval(oldtup, tupdesc, attnums[i], &isnull); - if (isnull) - return false; - - /* - * Get one attribute's newvalue. If it is NULL - they're not equal. - */ - newvalue = SPI_getbinval(newtup, tupdesc, attnums[i], &isnull); - if (isnull) + * Get one attribute's oldvalue and newvalue. If they are both NULL, + * the attribute is unchanged. If only one is NULL, it has changed. + */ + oldvalue = SPI_getbinval(oldtup, tupdesc, attnums[i], &isoldnull); + newvalue = SPI_getbinval(newtup, tupdesc, attnums[i], &isnewnull); + if (isoldnull || isnewnull) + { + if (isoldnull && isnewnull) + continue; return false; + } /* * Compare them with the appropriate equality operator. @@ -3821,13 +3818,13 @@ ri_KeysEqual(Relation rel, HeapTuple oldtup, HeapTuple newtup, /* ---------- - * ri_AllKeysUnequal - + * ri_AllKeysChanged - * - * Check if all key values in OLD and NEW are not equal. + * Check if all key values in OLD and NEW are changed. * ---------- */ static bool -ri_AllKeysUnequal(Relation rel, HeapTuple oldtup, HeapTuple newtup, +ri_AllKeysChanged(Relation rel, HeapTuple oldtup, HeapTuple newtup, const RI_ConstraintInfo *riinfo, bool rel_is_pk) { TupleDesc tupdesc = RelationGetDescr(rel); @@ -3850,28 +3847,28 @@ ri_AllKeysUnequal(Relation rel, HeapTuple oldtup, HeapTuple newtup, { Datum oldvalue; Datum newvalue; - bool isnull; - - /* - * Get one attribute's oldvalue. If it is NULL - they're not equal. - */ - oldvalue = SPI_getbinval(oldtup, tupdesc, attnums[i], &isnull); - if (isnull) - continue; + bool isoldnull; + bool isnewnull; /* - * Get one attribute's newvalue. If it is NULL - they're not equal. + * Get one attribute's oldvalue and newvalue. If they are both NULL, + * the attribute is unchanged. If only one is NULL, it has changed. */ - newvalue = SPI_getbinval(newtup, tupdesc, attnums[i], &isnull); - if (isnull) + oldvalue = SPI_getbinval(oldtup, tupdesc, attnums[i], &isoldnull); + newvalue = SPI_getbinval(newtup, tupdesc, attnums[i], &isnewnull); + if (isoldnull || isnewnull) + { + if (isoldnull && isnewnull) + return false; /* found two unchanged items */ continue; + } /* * Compare them with the appropriate equality operator. */ if (ri_AttributesEqual(eq_oprs[i], RIAttType(rel, attnums[i]), oldvalue, newvalue)) - return false; /* found two equal items */ + return false; /* found two unchanged items */ } return true; @@ -3879,17 +3876,17 @@ ri_AllKeysUnequal(Relation rel, HeapTuple oldtup, HeapTuple newtup, /* ---------- - * ri_OneKeyEqual - + * ri_OneKeyUnchanged - * - * Check if one key value in OLD and NEW is equal. Note column is indexed + * Check if one key value in OLD and NEW is unchanged. Note column is indexed * from zero. * - * ri_KeysEqual could call this but would run a bit slower. For + * ri_KeysUnchanged could call this but would run a bit slower. For * now, let's duplicate the code. * ---------- */ static bool -ri_OneKeyEqual(Relation rel, int column, HeapTuple oldtup, HeapTuple newtup, +ri_OneKeyUnchanged(Relation rel, int column, HeapTuple oldtup, HeapTuple newtup, const RI_ConstraintInfo *riinfo, bool rel_is_pk) { TupleDesc tupdesc = RelationGetDescr(rel); @@ -3897,7 +3894,8 @@ ri_OneKeyEqual(Relation rel, int column, HeapTuple oldtup, HeapTuple newtup, const Oid *eq_oprs; Datum oldvalue; Datum newvalue; - bool isnull; + bool isoldnull; + bool isnewnull; if (rel_is_pk) { @@ -3911,18 +3909,17 @@ ri_OneKeyEqual(Relation rel, int column, HeapTuple oldtup, HeapTuple newtup, } /* - * Get one attribute's oldvalue. If it is NULL - they're not equal. + * Get one attribute's oldvalue and newvalue. If they are both NULL, + * the attribute is unchanged. If only one is NULL, it has changed. */ - oldvalue = SPI_getbinval(oldtup, tupdesc, attnums[column], &isnull); - if (isnull) - return false; - - /* - * Get one attribute's newvalue. If it is NULL - they're not equal. - */ - newvalue = SPI_getbinval(newtup, tupdesc, attnums[column], &isnull); - if (isnull) + oldvalue = SPI_getbinval(oldtup, tupdesc, attnums[column], &isoldnull); + newvalue = SPI_getbinval(newtup, tupdesc, attnums[column], &isnewnull); + if (isoldnull || isnewnull) + { + if (isoldnull && isnewnull) + return true; return false; + } /* * Compare them with the appropriate equality operator.
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers