Re: NEW in after insert trugger contained incorrect data

From: Adrian Klaver <adrian(dot)klaver(at)aklaver(dot)com>
To: brilliantov(at)byterg(dot)ru, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>
Cc: "pgsql-general(at)postgresql(dot)org" <pgsql-general(at)postgresql(dot)org>
Subject: Re: NEW in after insert trugger contained incorrect data
Date: 2014-11-14 16:00:54
Message-ID: 54662736.4030903@aklaver.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general

On 11/14/2014 07:32 AM, Brilliantov Kirill Vladimirovich wrote:
> Albe Laurenz wrote on 11/14/2014 01:28 PM:
>>
>> You should post the table definition and the whole trigger; the error
>> message seems to refer to things you omitted in your quote.
>>
>> Yours,
>> Laurenz Albe
>>

Just approaching caffeine level required to follow this:)

>
> Table with original data trassa.cpu_load:
> CREATE TABLE trassa.cpu_load
> (
> id serial NOT NULL,
> device integer NOT NULL,
> created timestamp without time zone NOT NULL DEFAULT now(),
> device_timestamp timestamp without time zone NOT NULL,
> cpu smallint NOT NULL,
> value smallint NOT NULL,
> CONSTRAINT cpu_load_pk PRIMARY KEY (id),
> CONSTRAINT cpu_load_device FOREIGN KEY (device)
> REFERENCES trassa.devices (id) MATCH SIMPLE
> ON UPDATE NO ACTION ON DELETE NO ACTION,
> CONSTRAINT cpu_load_val CHECK (value >= 0 AND value <= 100)
> )
> WITH (
> OIDS=FALSE
> );

FYI, in the function below you have declared aliases for the function
arguments e.g. device_id integer. You can use those aliases in the
function instead of $*. It would make things easier to follow.

http://www.postgresql.org/docs/9.3/interactive/plpgsql-declarations.html#PLPGSQL-DECLARATION-PARAMETERS

>
> Function for save values in table trassa.cpu_Load:
> CREATE OR REPLACE FUNCTION trassa.update_cpu_load_list(device_id
> integer, device_timestamp integer, device_cpu smallint[],
> device_cpu_load smallint[])
> RETURNS boolean AS
> $BODY$
> DECLARE
> val_len SMALLINT DEFAULT array_length($3, 1);
> cmd TEXT DEFAULT 'INSERT INTO trassa.cpu_load (device, device_timestamp,
> cpu, value) VALUES';
> result SMALLINT;
> ts TIMESTAMP DEFAULT to_timestamp($2);
> BEGIN
> IF val_len = array_length($4, 1) THEN
> FOR i IN 1..val_len LOOP
> cmd = cmd || '(' ||
> $1::text ||
> ',''' || ts::text || ''',' ||
> $3[i]::text || ',' ||
> $4[i]::text || ')';
> IF i != val_len THEN
> cmd = cmd || ',';
> END IF;

I have not thought this all the way through, but I see a potential
problem with the test above. It is not clear to me which version of cmd
you are using nor what exactly it returns. You might want to put a
NOTICE in there to see what you are actually building.

Also you might want to take a look at this section of the docs:

http://www.postgresql.org/docs/9.3/interactive/plpgsql-control-structures.html#PLPGSQL-RECORDS-ITERATING

In particular the following forms:

FOR target IN EXECUTE text_expression ...

FOREACH target [ SLICE number ] IN ARRAY expression LOOP

> END LOOP;
> EXECUTE cmd;
> GET DIAGNOSTICS result = ROW_COUNT;
> IF result = val_len THEN
> RETURN TRUE;
> ELSE
> RETURN FALSE;
> END IF;
> ELSE
> RETURN FALSE;
> END IF;
> END;$BODY$
> LANGUAGE plpgsql VOLATILE SECURITY DEFINER
> COST 100;
>
> Table for save statistic trassa.cpu_load_stat:
> CREATE TABLE trassa.cpu_load_stat
> (
> id serial NOT NULL,
> device integer NOT NULL,
> cpu smallint NOT NULL,
> min_value smallint NOT NULL,
> min_device_timestamp timestamp without time zone NOT NULL,
> min_timestamp timestamp without time zone,
> avg_value smallint NOT NULL,
> avg_timestamp timestamp without time zone NOT NULL,
> max_value smallint NOT NULL,
> max_device_timestamp timestamp without time zone NOT NULL,
> max_timestamp timestamp without time zone,
> total_value bigint NOT NULL,
> total_count integer NOT NULL,
> CONSTRAINT cpu_load_stat_pk PRIMARY KEY (id),
> CONSTRAINT cpu_load_stat_device_fk FOREIGN KEY (device)
> REFERENCES trassa.devices (id) MATCH SIMPLE
> ON UPDATE NO ACTION ON DELETE NO ACTION,
> CONSTRAINT cpu_load_stat_avg_value_check CHECK (avg_value >= 0 AND
> avg_value <= 100),
> CONSTRAINT cpu_load_stat_max_value_check CHECK (max_value >= 0 AND
> max_value <= 100),
> CONSTRAINT cpu_load_stat_min_value_check CHECK (min_value >= 0 AND
> min_value <= 100)
> )
> WITH (
> OIDS=FALSE
> );
>
> Trigger for update trassa.cpu_load_stat, values from trassa.cpu_Load:
> CREATE OR REPLACE FUNCTION trassa.update_cpu_load_stat()
> RETURNS trigger AS
> $BODY$
> DECLARE
> line_id INTEGER DEFAULT 0;
> cpu_min_value SMALLINT DEFAULT 0;
> cpu_min_created_timestamp TIMESTAMP;
> cpu_min_device_timestamp TIMESTAMP;
> cpu_max_value SMALLINT DEFAULT 0;
> cpu_max_created_timestamp TIMESTAMP;
> cpu_max_device_timestamp TIMESTAMP;
> BEGIN
> SELECT id INTO line_id FROM trassa.cpu_load_stat
> WHERE device = NEW.device AND cpu = NEW.cpu;
> RAISE NOTICE USING MESSAGE = '*** START ***: ' || NEW;
> IF FOUND THEN
> RAISE NOTICE USING MESSAGE = '*** UPDATE ***: ID ' || line_id
> || ', data ' || NEW;
> SELECT created, device_timestamp, value
> INTO cpu_min_created_timestamp, cpu_min_device_timestamp,
> cpu_min_value
> FROM trassa.cpu_load
> WHERE trassa.cpu_load.device = NEW.device
> AND trassa.cpu_load.cpu = NEW.cpu
> ORDER BY value, created
> LIMIT 1;
>
> SELECT created, device_timestamp, value
> INTO cpu_max_created_timestamp, cpu_max_device_timestamp,
> cpu_max_value
> FROM trassa.cpu_load
> WHERE trassa.cpu_load.device = NEW.device
> AND trassa.cpu_load.cpu = NEW.cpu
> ORDER BY value DESC, created
> LIMIT 1;
>
> UPDATE trassa.cpu_load_stat
> SET min_value = cpu_min_value,
> min_device_timestamp = cpu_min_device_timestamp,
> min_timestamp = cpu_min_created_timestamp,
> avg_value = CEIL((total_value + NEW.value) /
> (total_count + 1)),
> avg_timestamp = NOW(),
> max_value = cpu_max_value,
> max_device_timestamp = cpu_max_device_timestamp,
> max_timestamp = cpu_max_created_timestamp,
> total_value = (total_value + NEW.value),
> total_count = (total_count + 1)
> WHERE id = line_id;
> RAISE NOTICE '*** END UPDATE ***';
> ELSE
> RAISE NOTICE USING MESSAGE = '*** INSERT ***: ' || NEW;
> INSERT INTO trassa.cpu_load_stat
> (device, cpu,
> min_value, min_device_timestamp, min_timestamp,
> avg_value, avg_timestamp,
> max_value, max_device_timestamp, max_timestamp,
> total_value, total_count)
> VALUES (NEW.device, NEW.cpu,
> NEW.value, NEW.device_timestamp, NOW(),
> NEW.value, NOW(),
> NEW.value, NEW.device_timestamp, NOW(),
> NEW.value, 1);
> RAISE NOTICE '*** END INSERT ***';
> END IF;
> RAISE NOTICE USING MESSAGE = '*** END ***: ' || TG_NAME;
> RETURN NULL;
> END;
> $BODY$
> LANGUAGE plpgsql VOLATILE SECURITY DEFINER
> COST 100;
>
> Trigger update_cpu_load_stat added to table trassa.cpu_load:
> CREATE TRIGGER update_cpu_load_stat_trigger
> AFTER INSERT
> ON trassa.cpu_load_stat
> FOR EACH ROW
> EXECUTE PROCEDURE trassa.update_cpu_load_stat();
>
> Thank you and excuse my big message.
>

--
Adrian Klaver
adrian(dot)klaver(at)aklaver(dot)com

In response to

Browse pgsql-general by date

  From Date Subject
Next Message Adrian Klaver 2014-11-14 16:09:50 Re: NEW in after insert trugger contained incorrect data
Previous Message Brilliantov Kirill Vladimirovich 2014-11-14 15:32:31 Re: NEW in after insert trugger contained incorrect data