Re: NEW in after insert trugger contained incorrect data

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.klaver@aklaver.com