Creating a trigger for child table insertion returns confusing error

Question

I am trying to write a trigger function that will input values into separate child tables, however I am getting an error I have not seen before.

Here is an example set up:

-- create initial table
CREATE TABLE public.testlog(
    id serial not null,
    col1 integer,
    col2 integer,
    col3 integer,
    name text
);

-- create child table
CREATE TABLE public.testlog_a (primary key(id)) INHERITS(public.testlog);

-- make trigger function for insert
CREATE OR REPLACE FUNCTION public.test_log() RETURNS trigger AS
$$
DECLARE
    qry text;
BEGIN
    qry := 'INSERT INTO public.testlog_' || NEW.name || ' SELECT ($1).*';

    EXECUTE qry USING NEW.*;

    RETURN OLD;
END
$$
LANGUAGE plpgsql VOLATILE SECURITY DEFINER;

-- add function to table
CREATE TRIGGER test_log_sorter BEFORE INSERT
ON public.testlog FOR EACH ROW
EXECUTE PROCEDURE public.test_log();

and the query:

INSERT INTO public.testlog (col1, col2, col3, name) values (1, 2, 3, 'a');

error message:

[Err] ERROR:  query "SELECT NEW.*" returned 5 columns
CONTEXT:  PL/pgSQL function test_log() line 7 at EXECUTE statement

5 columns is exactly what I am looking for it to return, so clearly there is something I am not understanding but the error message seems to make no sense.

Can anybody explain why I am getting this?

Adamina

Answer 1

Your solution fixes the passing of the row-typed NEW variable. However, you have a sneaky SQL-injection hole in your code, that's particularly dangerous in a SECURITY DEFINER function. User input must never be converted to SQL code unescaped.

Sanitize like this:

CREATE OR REPLACE FUNCTION trg_test_log()
  RETURNS trigger AS
$$
BEGIN
    EXECUTE 'INSERT INTO public.' || quote_ident('testlog_' || NEW.name)
         || ' SELECT ($1).*'
    USING NEW;

    RETURN NULL;
END
$$
LANGUAGE plpgsql SECURITY DEFINER;

Also:

• OLD is not defined in an INSERT trigger.
• You don't need a variable. Assignments are comparatively expensive in plpgsql.

Mokpo

Very good points, the "name" column was a placeholder because I was planning to dictate the child table using a sub select from a group of integers, the thought of injection didn't even occour to me. That said you can never be too careful about these things, and there's always the chance I might unthinkingly re-use the vulnerable code and get stung. – Adamina

@Lucas: An integer column would be safe of course, but the example was using text and the answer is for the general public. Anyway, like you said ... In passing, I fixed the malformed VALUES expression I had slipped in. – Mokpo

Answer 2

The EXECUTE qry USING NEW.* passes in the NEW.* as the arguments to the query. Since NEW.* returns five columns, the query should have $1, $2, $3, $4 and $5 in order to bind the five columns.

You are expecting a single argument ($1) which has five columns in it. I believe that if you change the the line to

EXECUTE qry USING NEW;

it will work as you expect.

Yoicks

Answer 3

With regards to Robert M. Lefkowitz' response, the answer is so simple: NEW as opposed to NEW.*

CREATE OR REPLACE FUNCTION public.test_log() RETURNS trigger AS
$$
DECLARE
    qry text;
BEGIN
    qry := 'INSERT INTO public.testlog_' || NEW.name || ' SELECT ($1).*';

    EXECUTE qry USING NEW;

    RETURN OLD;
END
$$
LANGUAGE plpgsql VOLATILE SECURITY DEFINER
COST 100;

thanks.

Adamina