1

A standard delete from foo_history where id = 123; returns the text:

DELETE 1

I have created a trigger procedure to replace the delete trigger, and instead of the delete, set an archive flag:

CREATE FUNCTION archive()
  RETURNS trigger AS $$
    DECLARE
      command text := '" SET timeEnd = current_timestamp WHERE id = $1';
    BEGIN
      EXECUTE 'UPDATE "' || TG_TABLE_NAME || command USING OLD.id;
      RETURN NULL;
    END;
  $$ LANGUAGE plpgsql;

Then created a view for the foo_history table called foo:

CREATE VIEW foo AS
SELECT id, timeStart from foo_history WHERE timeEnd IS NULL;

And added a trigger to the view:

CREATE TRIGGER foo_archive INSTEAD OF DELETE ON foo FOR EACH ROW EXECUTE PROCEDURE archive();

In other words, when deleting a foo record , set the timeEnd column instead - this way, the foo view only shows records without timeEnd set.

This works well:

delete from foo where id = 123

Does indeed set the timeEnd column. However, the output from this is:

DELETE 0

Whereas I really wanted it to say:

DELETE 1

Being an SQL novice and Postgres newbie I'm not sure how to change the archive function to do this.

I did try changing it to:

RETURN 1

But I get an error about I need to return a composite (whatever that means).

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
jmls
  • 2,879
  • 4
  • 34
  • 58

2 Answers2

3

You already found that you need to RETURN OLD in an INSTEAD OF trigger for a DELETE operation to make the row count in the returned command tag. The manual:

A row-level INSTEAD OF trigger should either return NULL to indicate that it did not modify any data from the view's underlying base tables, or it should return the view row that was passed in (the NEW row for INSERT and UPDATE operations, or the OLD row for DELETE operations). A nonnull return value is used to signal that the trigger performed the necessary data modifications in the view. This will cause the count of the number of rows affected by the command to be incremented.

Bold emphasis mine.

But you also have a sneaky SQL injection bug in your current trigger. TG_TABLE_NAME is the bare, unquoted name. Identifiers have to be treated as user input - and have to be quoted as necessary in any case. Quote the table name properly in dynamic SQL statements (or you may suffer consequences). One way is to use format():

CREATE OR REPLACE FUNCTION archive()
  RETURNS trigger AS
$func$
BEGIN
   EXECUTE format('UPDATE %I SET time_end = current_timestamp WHERE id = $1', TG_TABLE_NAME)
   USING OLD.id;
   RETURN OLD;
END
$func$  LANGUAGE plpgsql;

Related:

I also used time_end instead of timeEnd, since my standing advice is to avoid CaMeL case identifiers in Postgres. Related:

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
  • many thanks indeed. I did overlook the entry in the manual for some reason .. and the observation about sql injection was spot on. – jmls Mar 29 '18 at 06:36
  • Thank you!! I had tried both `return new` and `return null` but hadn't thought to try `return old`. This got me unstuck. – WillB3 Feb 23 '22 at 16:52
0

and, of course, the second I post this, I have an idea which seems to work ..

instead of RETURN NULL, use RETURN OLD

sorry ...

jmls
  • 2,879
  • 4
  • 34
  • 58