1

I am trying to create a dynamic function to use for setting up triggers.

CREATE OR REPLACE FUNCTION device_bid_modifiers_count_per()
  RETURNS TRIGGER AS
$$
  DECLARE
    devices_count INTEGER;
    table_name    regclass := TG_ARGV[0];
    column_name   VARCHAR  := TG_ARGV[1];
  BEGIN
    LOCK TABLE device_types IN EXCLUSIVE MODE;
    EXECUTE format('LOCK TABLE %s IN EXCLUSIVE MODE', table_name);

    SELECT INTO devices_count device_types_count();

    IF TG_OP = 'DELETE' THEN
      SELECT format(
        'PERFORM validate_bid_modifiers_count(%s, %s, OLD.%s, %s)',
        table_name,
        column_name,
        column_name,
        devices_count
      );
    ELSE
      SELECT format(
        'PERFORM validate_bid_modifiers_count(%s, %s, NEW.%s, %s)',
        table_name,
        column_name,
        column_name,
        devices_count
      );
    END IF;
    RETURN NEW;
  END;
$$ LANGUAGE plpgsql;

My issue is with the execution of the dynamic function validate_bid_modifiers_count(). Currently it throws:

ERROR:  query has no destination for result data
HINT:  If you want to discard the results of a SELECT, use PERFORM instead.
CONTEXT:  PL/pgSQL function device_bid_modifiers_count_per() line 21 at SQL statement

I can't really wrap my head around this. I understand that format() returns the correct string of function call with arguments. How do I fix this and make it work?

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
Andrey Deineko
  • 51,333
  • 10
  • 112
  • 145
  • I tried `EXECUTE format('SELECT...)` and different combinations of `EXECUTE/PERFORM/SELECT` – Andrey Deineko Mar 19 '19 at 16:09
  • Please show the errors when you tried to use `EXECUTE` and `PERFORM`. I'm not a PLPGSQL person, but you certainly don't want to `SELECT` the code you want to run. The errors may elaborate on whether such dynamic SQL is even possible *(For example, I doubt that referencing `NEW` and `OLD` are even possible within the scope of the dynamically executed code.)* – MatBailie Mar 19 '19 at 16:14

2 Answers2

2

This should do it:

CREATE OR REPLACE FUNCTION device_bid_modifiers_count_per()
  RETURNS TRIGGER AS
$func$
DECLARE
   devices_count int      := device_types_count();
   table_name    regclass := TG_ARGV[0];
   column_name   text     := TG_ARGV[1];
BEGIN
   LOCK TABLE device_types IN EXCLUSIVE MODE;
   EXECUTE format('LOCK TABLE %s IN EXCLUSIVE MODE', table_name);

   IF TG_OP = 'DELETE' THEN
      PERFORM validate_bid_modifiers_count(table_name
                                         , column_name
                                         , (row_to_json(OLD) ->> column_name)::bigint
                                         , devices_count);
   ELSE
      PERFORM validate_bid_modifiers_count(table_name
                                         , column_name
                                         , (row_to_json(NEW) ->> column_name)::bigint
                                         , devices_count);
   END IF;

   RETURN NEW;
END
$func$  LANGUAGE plpgsql;

The immediate cause for the error message was the outer SELECT. Without target, you need to replace it with PERFORM in plpgsql. But the inner PERFORM in the query string passed to EXECUTE was wrong, too. PERFORM is a plpgsql command, not valid in an SQL string passed to EXECUTE, which expects SQL code. You have to use SELECT there. Finally OLD and NEW are not visible inside EXECUTE and would each raise an exception of their own the way you had it. All issues are fixed by dropping EXECUTE.

A simple and fast way to get the value of a dynamic column name from the row types OLD and NEW: cast to json, then you can parameterize the key name like demonstrated. Should be a bit simpler and faster than the alternative with dynamic SQL - which is possible as well, like:

  ...
  EXECUTE format('SELECT validate_bid_modifiers_count(table_name
                                                    , column_name
                                                    , ($1.%I)::bigint
                                                    , devices_count)', column_name)
  USING OLD;
  ...

Related:

Aside: Not sure why you need the heavy locks.

Aside 2: Consider writing a separate trigger function for each trigger instead. More noisy DDL, but simpler and faster to execute.

Erwin Brandstetter
  • 605,456
  • 145
  • 1,078
  • 1,228
  • Hey Erwin, first of all let me thank you for all your answers on SO. I have recently started to deal with native SQL (I was always shamelessly using ORM) and if I learned anything at all - it was usually based on your answers. Huge thank you for that – Andrey Deineko Mar 20 '19 at 06:01
  • On the topic: I swear this is exactly what I had initially (except for the second lock statement - I had it as LOCK TABLE table_name IN EXCLUSIVE MODE. Bun then I started to get errors which brought me to the "need" of dynamic eveluation. Sorry for the wrong wording. Anyway, the error with that solution is following: `ERROR: record "new" has no field "column_name" CONTEXT: SQL statement "SELECTvalidate_bid_modifiers_count(table_name, column_name, NEW.column_name, devices_count)" PL/pgSQL function device_bid_modifiers_count_per() line 15 at PERFORM` – Andrey Deineko Mar 20 '19 at 08:13
  • @AndreyDeineko: Updated to cover the dynamic column name I had missed at first. – Erwin Brandstetter Mar 20 '19 at 18:39
  • Thank you for the updated answer! If it is indeed making any difference in the speed of the execution then I will go for 2 separate trigger functions. It's just that their definition will be literally identical except for a `table_name` and `column_name` arguments. I wanted a single function because at least in Ruby we are usually aiming for a DRY code. Regarding locks - the function depends on `device_types` so I lock it not to change during the function execution, and regarding the table, the function applies to - well, maybe it is not needed indeed. Or `SHARE` mode, not sure – Andrey Deineko Mar 21 '19 at 06:49
  • I have updated the answer just for the bravity of correct type passing – Andrey Deineko Mar 21 '19 at 07:21
0

As I pointed out in the comment to Erwin Brandstetter's answer, initially I have an almost identical solution.

But issue was that I was getting the error

ERROR: record "new" has no field "column_name"
CONTEXT: SQL statement "SELECT validate_bid_modifiers_count(table_name, column_name, NEW.column_name, devices_count)"
PL/pgSQL function device_bid_modifiers_count_per() line 15 at PERFORM

This is why I thought I needed a way to dynamically evaluate things.

Currently got this working with the following still ugly looking to me solution (ugly because I don't like 2 IF statements, I would like it to be super dynamic, but maybe I am asking for too much):

CREATE OR REPLACE FUNCTION device_bid_modifiers_count_per()
  RETURNS TRIGGER AS
$func$
  DECLARE
    row           RECORD;
    table_name    regclass := TG_ARGV[0];
    column_name   text := TG_ARGV[1];
    devices_count INTEGER;

  BEGIN
    LOCK TABLE device_types IN EXCLUSIVE MODE;
    EXECUTE format('LOCK TABLE %s IN EXCLUSIVE MODE', table_name);

    devices_count := device_types_count();

    IF TG_OP = 'DELETE' THEN
      row := OLD;
    ELSE
      row := NEW;
    END IF;

    IF column_name = 'campaign_id' THEN
      PERFORM validate_bid_modifiers_count(table_name, column_name, row.campaign_id, devices_count);
    ELSIF column_name = 'adgroup_id' THEN
      PERFORM validate_bid_modifiers_count(table_name, column_name, row.adgroup_id, devices_count);
    ELSE
      RAISE EXCEPTION 'invalid_column_name %', column_name;
    END IF;
    RETURN NEW;
  END;
$func$ LANGUAGE plpgsql;

I am open to more robust solution suggestions.

Basically, the second condition kind'a almost defeats the purpose of having a single function, I could have at this point as well split it into two functions. Because the goal is to define multiple (2) triggers using this function (providing arguments to it).

Andrey Deineko
  • 51,333
  • 10
  • 112
  • 145