Re: The suppress_redundant_updates_trigger() works incorrectly

Andrew Dunstan wrote:
>
>
> KaiGai Kohei wrote:
>> *** 80,88 ****
>>            HeapTupleHeaderGetNatts(oldheader)) &&
>>           ((newheader->t_infomask & ~HEAP_XACT_MASK) ==
>> (oldheader->t_infomask & ~HEAP_XACT_MASK)) &&
>> !         memcmp(((char *)newheader) + offsetof(HeapTupleHeaderData,
>> t_bits),
>> !                ((char *)oldheader) + offsetof(HeapTupleHeaderData,
>> t_bits),
>> !                newtuple->t_len - offsetof(HeapTupleHeaderData,
>> t_bits)) == 0)
>>       {
>>           /* ... then suppress the update */
>>           rettuple = NULL;
>> --- 86,94 ----
>>            HeapTupleHeaderGetNatts(oldheader)) &&
>>           ((newheader->t_infomask & ~HEAP_XACT_MASK) ==
>> (oldheader->t_infomask & ~HEAP_XACT_MASK)) &&
>> !         memcmp(((char *)newheader) + newheader->t_hoff,
>> !                ((char *)oldheader) + oldheader->t_hoff,
>> !                newtuple->t_len - newheader->t_hoff) == 0)
>>       {
>>           /* ... then suppress the update */
>>           rettuple = NULL;
>>
>
> Wouldn't this omit comparing the null bitmap?

Oops, I added the comparison of null bitmap here.

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>
*** base/src/backend/utils/adt/trigfuncs.c.orig    2008-11-06 10:24:39.000000000 +0900
--- base/src/backend/utils/adt/trigfuncs.c    2008-11-06 11:26:27.000000000 +0900
***************
*** 62,92 ****
      newheader = newtuple->t_data;
      oldheader = oldtuple->t_data;

-     /*
-      * We are called before the OID, if any, has been transcribed from the
-      * old tuple to the new (in heap_update).  To avoid a bogus compare
-      * failure, copy the OID now.  But check that someone didn't already put
-      * another OID value into newtuple.  (That's not actually possible at
-      * present, but maybe someday.)
-      */
-      if (trigdata->tg_relation->rd_rel->relhasoids &&
-         !OidIsValid(HeapTupleHeaderGetOid(newheader)))
-         HeapTupleHeaderSetOid(newheader, HeapTupleHeaderGetOid(oldheader));
-
      /* if the tuple payload is the same ... */
      if (newtuple->t_len == oldtuple->t_len &&
          newheader->t_hoff == oldheader->t_hoff &&
          (HeapTupleHeaderGetNatts(newheader) ==
           HeapTupleHeaderGetNatts(oldheader)) &&
          ((newheader->t_infomask & ~HEAP_XACT_MASK) ==
!          (oldheader->t_infomask & ~HEAP_XACT_MASK)) &&
!         memcmp(((char *)newheader) + offsetof(HeapTupleHeaderData, t_bits),
!                ((char *)oldheader) + offsetof(HeapTupleHeaderData, t_bits),
!                newtuple->t_len - offsetof(HeapTupleHeaderData, t_bits)) == 0)
      {
          /* ... then suppress the update */
          rettuple = NULL;
      }

      return PointerGetDatum(rettuple);
  }
--- 62,104 ----
      newheader = newtuple->t_data;
      oldheader = oldtuple->t_data;

      /* if the tuple payload is the same ... */
      if (newtuple->t_len == oldtuple->t_len &&
          newheader->t_hoff == oldheader->t_hoff &&
          (HeapTupleHeaderGetNatts(newheader) ==
           HeapTupleHeaderGetNatts(oldheader)) &&
          ((newheader->t_infomask & ~HEAP_XACT_MASK) ==
!          (oldheader->t_infomask & ~HEAP_XACT_MASK)))
      {
+         if (HeapTupleHasNulls(newtuple) &&
+             memcmp(newheader->t_bits, oldheader->t_bits,
+                    BITMAPLEN(HeapTupleHeaderGetNatts(newheader))) != 0)
+             goto skip;
+
+         /*
+          * We are called before the OID, if any, has been transcribed from
+          * the old tuple to the new (in heap_update). To avoid a bogus compare
+          * failure, compare the OID new. But check that someone didn't already
+          * put another OID value into newtuple.  (That's not actually possible
+          * at present, but maybe someday.)
+          */
+         if (OidIsValid(HeapTupleGetOid(newtuple))
+             && HeapTupleGetOid(newtuple) != HeapTupleGetOid(oldtuple))
+             goto skip;
+ #if 0
+         if (OidIsValid(HeapTupleGetSecurity(newtuple))
+             && HeapTupleGetSecurity(newtuple) != HeapTupleGetSecurity(oldtuple))
+             goto skip;
+ #endif
+         if (memcmp(((char *)newheader) + newheader->t_hoff,
+                    ((char *)oldheader) + oldheader->t_hoff,
+                    newtuple->t_len - newheader->t_hoff) != 0)
+             goto skip;
+
          /* ... then suppress the update */
          rettuple = NULL;
      }
+ skip:

      return PointerGetDatum(rettuple);
  }