Postgresql – Modify function to use it in multiple triggers

dynamic-sqlinsertplpgsqlpostgresqltrigger

I have tables that have almost the same columns. The first table has the current values and the other table has the old values and an additional column to store the date data is inserted. The second table is named as the first table with the prefix _bit. Example:

CREATE TABLE x (
  id serial PRIMARY KEY,
  f1 integer NOT NULL,
  f2 character varying(10) NOT NULL,
  cuando timestamp without time zone NOT NULL DEFAULT now()
);

CREATE TABLE _bitx (
  id integer,
  f1 integer,
  f2 character varying(10),
  cuando timestamp without time zone,
  cuandobit timestamp without time zone DEFAULT now()
);

I have a trigger function that copies the OLD values of a table to another:

CREATE FUNCTION bitacoratabla() RETURNS trigger AS $$
   BEGIN
      INSERT INTO _bitx VALUES((OLD).*);
      RETURN NEW;
   END;
$$ LANGUAGE plpgsql

The trigger:

CREATE TRIGGER trigbitx
  BEFORE UPDATE
  ON public.x
  FOR EACH ROW
  EXECUTE PROCEDURE public.bitacoratabla();

All this works fine but I must do this for multiple tables that follow the same rules.

I modified the function in order to create the name of the table where I will insert data but I am having problems with that and with (OLD).*

CREATE FUNCTION bitacoratabla() RETURNS trigger AS $$
   DECLARE
      tabla text := '_bit' || quote_ident(TG_TABLE_NAME);

   BEGIN
      EXECUTE 'INSERT INTO ' || tabla || ' VALUES(' || (OLD).* || ')';
      RETURN NEW;
   END;
$$ LANGUAGE plpgsql

I am new to PL/pgSQL and thought I did not have to do more. Any ideas?

UPDATE

As all you suggest it is better to use format() and pass values with USING keyword. Thank you. This is what I did:

CREATE OR REPLACE FUNCTION bitacoratabla() RETURNS TRIGGER 
AS $$
   BEGIN
      EXECUTE format('INSERT INTO %I SELECT $1.*', '_bit' || TG_TABLE_NAME) USING OLD;
      RETURN NEW;
   END 
$$ LANGUAGE PLPGSQL;

Best Answer

Answers so far still suffer from one or the other shortcoming.

  1. Use the USING clause of EXECUTE to pass values. Do not concatenate the text representation of values into the SQL string. Expensive and error-prone.

  2. Properly escape all identifiers when used in dynamic SQL to defend against sneaky errors with double quoted identifiers and possible SQL injection. Even if you think you do not need it. Do it on principal.

    But this would be an error waiting to happen:

    '_bit' || quote_ident(TG_TABLE_NAME)

    For a table name "MyTable" this would produce _bit"MyTable" - syntactical nonsense. It must be:

    quote_ident('_bit' || TG_TABLE_NAME)

    Or use format('... %I ...', '_bit' || TG_TABLE_NAME)

The only correct solution is what you added to your question yourself:

CREATE OR REPLACE FUNCTION bitacoratabla()
  RETURNS trigger AS
$func$
BEGIN
   EXECUTE format('INSERT INTO %I SELECT $1.*', '_bit' || TG_TABLE_NAME)
   USING OLD;

   RETURN NEW;
END
$func$  LANGUAGE plpgsql;

Detailed explanation:

But why does it work with an additional column?

the other table has the old values and an additional column to store the date data

This happens to work as long as the additional column in the target table comes last because, quoting the manual on INSERT:

If no list of column names is given at all, the default is all the columns of the table in their declared order; or the first N column names, if there are only N columns supplied by the VALUES clause or query.

So you better be sure the additional column(s) come last in the table definition of the target table, and all other columns are listed in matching order, now and in all future - or you must add an explicit target column list (which would prevent a generic function to begin with).

Related Question