Re: The suppress_redundant_updates_trigger() works incorrectly

Andrew Dunstan wrote:
> KaiGai Kohei wrote:
>
>> Hi,
>>
>> The suppress_redundant_updates_trigger() works incorrectly
>> on the table defined with "WITH_OIDS" option.
>>
>>
>>
>
> Good catch!
>
> I think ideally we'd just adjust the comparison to avoid the system
> attributes.
>
> I'll have a look to see if it can be done simply.
>
>
>

The attached patch sets the OID to InvalidOid for the duration of the
memcmp if the HEAP_HASOID flag is set, and restores it afterwards.

That seems to handle the problem.

There's also an added regression test for the Oids case.

If there's no objection I'll apply this shortly.

cheers

andrew
Index: src/backend/utils/adt/trigfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/trigfuncs.c,v
retrieving revision 1.2
diff -c -r1.2 trigfuncs.c
*** src/backend/utils/adt/trigfuncs.c    4 Nov 2008 00:29:39 -0000    1.2
--- src/backend/utils/adt/trigfuncs.c    5 Nov 2008 17:30:26 -0000
***************
*** 30,35 ****
--- 30,36 ----
      TriggerData *trigdata = (TriggerData *) fcinfo->context;
      HeapTuple   newtuple, oldtuple, rettuple;
      HeapTupleHeader newheader, oldheader;
+     Oid oldoid = InvalidOid;

      /* make sure it's called as a trigger */
      if (!CALLED_AS_TRIGGER(fcinfo))
***************
*** 62,67 ****
--- 63,74 ----
      newheader = newtuple->t_data;
      oldheader = oldtuple->t_data;

+      if (oldheader->t_infomask & HEAP_HASOID)
+     {
+         oldoid = HeapTupleHeaderGetOid(oldheader);
+         HeapTupleHeaderSetOid(oldheader, InvalidOid);
+     }
+
      /* if the tuple payload is the same ... */
      if (newtuple->t_len == oldtuple->t_len &&
          newheader->t_hoff == oldheader->t_hoff &&
***************
*** 76,81 ****
--- 83,93 ----
          /* ... then suppress the update */
          rettuple = NULL;
      }
+      else if (oldheader->t_infomask & HEAP_HASOID)
+     {
+         HeapTupleHeaderSetOid(oldheader, oldoid);
+     }
+

      return PointerGetDatum(rettuple);
  }
Index: src/test/regress/expected/triggers.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/triggers.out,v
retrieving revision 1.25
diff -c -r1.25 triggers.out
*** src/test/regress/expected/triggers.out    3 Nov 2008 20:17:20 -0000    1.25
--- src/test/regress/expected/triggers.out    5 Nov 2008 17:30:27 -0000
***************
*** 542,551 ****
--- 542,559 ----
      f1    text,
      f2 int,
      f3 int);
+ CREATE TABLE min_updates_test_oids (
+     f1    text,
+     f2 int,
+     f3 int) WITH OIDS;
  INSERT INTO min_updates_test VALUES ('a',1,2),('b','2',null);
+ INSERT INTO min_updates_test_oids VALUES ('a',1,2),('b','2',null);
  CREATE TRIGGER z_min_update
  BEFORE UPDATE ON min_updates_test
  FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
+ CREATE TRIGGER z_min_update
+ BEFORE UPDATE ON min_updates_test_oids
+ FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
  \set QUIET false
  UPDATE min_updates_test SET f1 = f1;
  UPDATE 0
***************
*** 553,558 ****
--- 561,572 ----
  UPDATE 2
  UPDATE min_updates_test SET f3 = 2 WHERE f3 is null;
  UPDATE 1
+ UPDATE min_updates_test_oids SET f1 = f1;
+ UPDATE 0
+ UPDATE min_updates_test_oids SET f2 = f2 + 1;
+ UPDATE 2
+ UPDATE min_updates_test_oids SET f3 = 2 WHERE f3 is null;
+ UPDATE 1
  \set QUIET true
  SELECT * FROM min_updates_test;
   f1 | f2 | f3
***************
*** 561,564 ****
--- 575,586 ----
   b  |  3 |  2
  (2 rows)

+ SELECT * FROM min_updates_test_oids;
+  f1 | f2 | f3
+ ----+----+----
+  a  |  2 |  2
+  b  |  3 |  2
+ (2 rows)
+
  DROP TABLE min_updates_test;
+ DROP TABLE min_updates_test_oids;
Index: src/test/regress/sql/triggers.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/triggers.sql,v
retrieving revision 1.14
diff -c -r1.14 triggers.sql
*** src/test/regress/sql/triggers.sql    3 Nov 2008 20:17:21 -0000    1.14
--- src/test/regress/sql/triggers.sql    5 Nov 2008 17:30:27 -0000
***************
*** 424,435 ****
--- 424,446 ----
      f2 int,
      f3 int);

+ CREATE TABLE min_updates_test_oids (
+     f1    text,
+     f2 int,
+     f3 int) WITH OIDS;
+
  INSERT INTO min_updates_test VALUES ('a',1,2),('b','2',null);

+ INSERT INTO min_updates_test_oids VALUES ('a',1,2),('b','2',null);
+
  CREATE TRIGGER z_min_update
  BEFORE UPDATE ON min_updates_test
  FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();

+ CREATE TRIGGER z_min_update
+ BEFORE UPDATE ON min_updates_test_oids
+ FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
+
  \set QUIET false

  UPDATE min_updates_test SET f1 = f1;
***************
*** 438,446 ****
--- 449,467 ----

  UPDATE min_updates_test SET f3 = 2 WHERE f3 is null;

+ UPDATE min_updates_test_oids SET f1 = f1;
+
+ UPDATE min_updates_test_oids SET f2 = f2 + 1;
+
+ UPDATE min_updates_test_oids SET f3 = 2 WHERE f3 is null;
+
  \set QUIET true

  SELECT * FROM min_updates_test;

+ SELECT * FROM min_updates_test_oids;
+
  DROP TABLE min_updates_test;

+ DROP TABLE min_updates_test_oids;
+