3

I'm starting and trying some things in a new database, and am encountering a problem. I'm new in PostgreSQL.

I'm trying to create a history for the change of values in a column of a users table. The idea is simple. Whenever there is an update, a new record is inserted in another table (which represents the history).

DROP FUNCTION IF EXISTS LOCA_APP.FUNC_HISTORICO_MOD_USUARIOS() CASCADE;
CREATE OR REPLACE FUNCTION LOCA_APP.FUNC_HISTORICO_MOD_USUARIOS() RETURNS TRIGGER
AS $$ 
BEGIN
    EXECUTE 'INSERT INTO LOCA_APP.TB_MODIFICACOES (
        MOD_MOMENTO ,             -- Translated to: Moment
        MOD_VALOR_ANTERIOR ,      -- Translated to: Old value
        MOD_VALOR_ATUAL ,         -- Translated to: New value
        MOD_USUARIO ,             -- Translated to: User (ID)
        MOD_DADO)                 -- Translated to: Data (Column Name - ID)
    VALUES(
        now() , 
        OLD.' || TG_ARGV[0] || ' , 
        NEW.' || TG_ARGV[0] || ' , 
        '|| TG_RELID || ' ,
        (SELECT DAD_ID FROM LOCA_APP.TB_DADOS WHERE DAD_NOME ILIKE ''' || TG_ARGV[0] || ''') );';
END $$ LANGUAGE plpgsql;

DROP TRIGGER IF EXISTS TRIG_HISTORICO_USU_ID ON LOCA_APP.TB_USUARIOS CASCADE;
CREATE TRIGGER TRIG_HISTORICO_USU_ID AFTER UPDATE OF USU_ID ON LOCA_APP.TB_USUARIOS
FOR EACH ROW EXECUTE PROCEDURE LOCA_APP.FUNC_HISTORICO_MOD_USUARIOS('USU_ID');

Note: EXECUTE comments were put to better understanding here. The original code does not have these comments.

The pgAdmin is telling me:

ERROR:  missing FROM-clause entry for table "old"
LINE 9:   OLD.USU_NASCIMENTO , 
          ^
QUERY:  INSERT INTO LOCA_APP.TB_MODIFICACOES (
      MOD_MOMENTO ,
      MOD_VALOR_ANTERIOR ,
      MOD_VALOR_ATUAL ,
      MOD_USUARIO , 
      MOD_DADO)
  VALUES(
      now() , 
      OLD.USU_NASCIMENTO , 
      NEW.USU_NASCIMENTO , 
      22664 ,
      (SELECT DAD_ID FROM LOCA_APP.TB_DADOS WHERE DAD_NOME ILIKE 'USU_NASCIMENTO') );
CONTEXT:  PL/pgSQL function loca_app.func_historico_mod_usuarios() line 3 at EXECUTE

********** Error **********

ERROR: missing FROM-clause entry for table "old"
SQL state: 42P01
Context: PL/pgSQL function loca_app.func_historico_mod_usuarios() line 3 at EXECUTE

I have a single schema named LOCA_APP in my database, and my PostgreSQL version is 9.5.

Can anyone explain what's wrong?

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
Loa
  • 2,117
  • 3
  • 21
  • 45

2 Answers2

6

This is how your trigger function would work properly:

CREATE OR REPLACE FUNCTION loca_app.func_historico_mod_usuarios()
  RETURNS trigger AS
$func$
BEGIN
   EXECUTE format(
      'INSERT INTO loca_app.tb_modificacoes
              (mod_momento, mod_valor_anterior, mod_valor_atual, mod_usuario, mod_dado)
       VALUES (now()      , $1.%1$I           , $2.%1$I        , $3         , $4)

              )', TG_ARGV[0])
   USING OLD, NEW, TG_RELID
      , (SELECT dad_id FROM loca_app.tb_dados
         WHERE  dad_nome = TG_ARGV[0]  -- cast? see blow
         LIMIT  1);

   RETURN NULL;  -- only good for AFTER trigger
END
$func$ LANGUAGE plpgsql;

Major points

  • Pass the special row values OLD and NEW as well as TG_RELID as values to EXECUTE with the USING clause. You may have to cast TG_RELID to a fitting data type. The table definition of tb_modificacoes is undisclosed. Or you really want something else here. See below.
    $1, $2 and $3 in the SQL string passed to EXECUTE refer to expressions in the USING clause, not to function parameters, which can be referenced with the same positional syntax in the function body outside EXECUTE.

  • Concatenate your dynamic SQL command using format(). Much cleaner and safer. Quote and escape identifiers, code and values properly! %1$I and %1$L are format specifiers for format(). Read the manual for details.

  • Correct case is required! Your convention to spell identifiers with upper case letters makes sense in Oracle, where unquoted identifiers are converted to upper-case letters. It's not useful in Postgres, where everything is folded to lower case instead:

  • Don't use ILIKE in DAD_NOME ILIKE 'USU_NASCIMENTO'. Postgres identifiers are case sensitive. You could have multiple matching values in dad_nome. Use = instead and pass identifiers spelled correctly. And make sure dad_nome is defined unique. See below.

  • Your comment says: MOD_USUARIO , -- Translated to: User (ID). But that's not what you pass. The manual:

    TG_RELID

    Data type oid; the object ID of the table that caused the trigger invocation.

    You may want to use current_user or session_user instead:

  • You can remove LIMIT 1 from the subquery if dad_nome is defined UNIQUE. Else you need to decide which row to pick in case of a tie - with ORDER BY.

  • Trigger functions are required to terminate with a RETURN statement. Might as well be RETURN NULL for an AFTER trigger. The manual:

    The return value is ignored for row-level triggers fired after an operation, and so they can return NULL.

Related:

Aside: While you are new to Postgres you might want to use this kind of advanced dynamic SQL carefully. You need to understand what you are doing.

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
2

It is your use of EXECUTE here that's causing the problem. When you prepare the statement in the way that you have done OLD becomes a literal instead of being a variable that holds a row of data as it would be outside the preparation.

As I see it you need execute because you are doing this:

    OLD.' || TG_ARGV[0] || ' , 
    NEW.' || TG_ARGV[0] || ' , 

Is it really needed? can't you just use

    OLD.USU_NASCIMENTO , 
    NEW.USU_NASCIMENTO , 

and be done with it? This way you avoid the need to use execute and OLD and NEW will behave as rows.

e4c5
  • 52,766
  • 11
  • 101
  • 134
  • I think it's not possible. I have a trigger for each modified column. So I need the column name with its old and new value. The intention is to persist which column has been modified, and store the previous and current value. :/ – Loa May 21 '16 at 02:13
  • be that as it may, the question is what was the error with these statements and the answer is the use of execute and OLD. As for any other way of doing it, I suppose that will have to be discussed in a separate question – e4c5 May 21 '16 at 02:34