On Sun, Feb 12, 2012 at 7:36 AM, Vik Reykja <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
