Related to bug #6123, Wisconsin Courts are now using triggers with
the workaround to be safe with the patch put forward by Tom, even
though they are still running with the earlier patch proposal by me
(which tolerates an UPDATE or DELETE of a row being deleted).  The
general structure of the BEFORE DELETE triggers with cascading logic
was like this:

DECLARE
return_value parenttable;
BEGIN
return_value := OLD;

DELETE FROM childtable1
WHERE <child of parent>;
IF FOUND THEN
return_value := NULL;
END IF;

DELETE FROM childtablen
WHERE <child of parent>;
IF FOUND THEN
return_value := NULL;
END IF;

IF return_value IS NULL THEN
DELETE FROM parent
WHERE <primary key matches OLD>;
END IF;

RETURN return_value;
END;

This did not work for cases where the AFTER DELETE trigger performed
an action which was not idempotent because, while return_value was
NULL enough to enter that last IF clause, it was not NULL enough to
prevent the DELETE attempt and fire subsequent triggers.  The
assignment of NULL to a variable with a record type doesn't assign a
"simple" NULL, but a record with NULL in each element.  It seems
like a POLA violation that:

return_value := NULL;
RETURN return_value;

behaves differently than:

RETURN NULL;

We've fixed the afflicted trigger functions by adding a RETURN NULL;
inside that last IF block, but:

- Is this behavior required by standard?
- If not, do we want to keep this behavior?
- If we keep this behavior, should the triggering operation be
suppressed when (NOT return_value IS NOT NULL) instead of when
(return_value IS NOT DISTINCT FROM NULL)?

-Kevin

Search Discussions

  • Tom Lane at Sep 20, 2012 at 9:31 pm

    "Kevin Grittner" <kgrittn@mail.com> writes:
    ... This did not work for cases where the AFTER DELETE trigger performed
    an action which was not idempotent because, while return_value was
    NULL enough to enter that last IF clause, it was not NULL enough to
    prevent the DELETE attempt and fire subsequent triggers. The
    assignment of NULL to a variable with a record type doesn't assign a
    "simple" NULL, but a record with NULL in each element.
    I believe that this is forced by plpgsql's implementation. IIRC, a
    declared variable of a named composite type (not RECORD) is implemented
    as a "row" structure, meaning it actually consists of a separate plpgsql
    variable for each column. So there's no physical way for it to represent
    a "simple NULL" composite value.

    I've suggested in the past that we might want to go over to treating
    such variables more like RECORD, ie the representation is always a
    HeapTuple. I'm not sure what the performance tradeoffs would be ---
    some things would get faster and others slower, probably, since field
    access would be more work but conversion to/from HeapTuple would get far
    cheaper.
    - If we keep this behavior, should the triggering operation be
    suppressed when (NOT return_value IS NOT NULL) instead of when
    (return_value IS NOT DISTINCT FROM NULL)?
    Can't do that, because it would break the perfectly-legitimate case
    where the trigger is trying to process a row of all nulls.

    regards, tom lane
  • Pavel Stehule at Sep 20, 2012 at 9:39 pm

    2012/9/20 Tom Lane <tgl@sss.pgh.pa.us>:
    "Kevin Grittner" <kgrittn@mail.com> writes:
    ... This did not work for cases where the AFTER DELETE trigger performed
    an action which was not idempotent because, while return_value was
    NULL enough to enter that last IF clause, it was not NULL enough to
    prevent the DELETE attempt and fire subsequent triggers. The
    assignment of NULL to a variable with a record type doesn't assign a
    "simple" NULL, but a record with NULL in each element.
    I believe that this is forced by plpgsql's implementation. IIRC, a
    declared variable of a named composite type (not RECORD) is implemented
    as a "row" structure, meaning it actually consists of a separate plpgsql
    variable for each column. So there's no physical way for it to represent
    a "simple NULL" composite value.

    I've suggested in the past that we might want to go over to treating
    such variables more like RECORD, ie the representation is always a
    HeapTuple.
    I had same idea when I worked on SQL/PSM - but there is significant
    difference in performance (probably less in real tasks)

    I'm not sure what the performance tradeoffs would be ---
    some things would get faster and others slower, probably, since field
    access would be more work but conversion to/from HeapTuple would get far
    cheaper.
    when fields are fix length, then field's update is significantly
    faster then when RECORD is used
    - If we keep this behavior, should the triggering operation be
    suppressed when (NOT return_value IS NOT NULL) instead of when
    (return_value IS NOT DISTINCT FROM NULL)?
    Can't do that, because it would break the perfectly-legitimate case
    where the trigger is trying to process a row of all nulls.

    regards, tom lane


    --
    Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
    To make changes to your subscription:
    http://www.postgresql.org/mailpref/pgsql-hackers
  • Tom Lane at Sep 20, 2012 at 9:55 pm

    Pavel Stehule writes:
    2012/9/20 Tom Lane <tgl@sss.pgh.pa.us>:
    I'm not sure what the performance tradeoffs would be ---
    some things would get faster and others slower, probably, since field
    access would be more work but conversion to/from HeapTuple would get far
    cheaper.
    when fields are fix length, then field's update is significantly
    faster then when RECORD is used
    [ shrug... ] Maybe not if we put a little effort into it. It would be
    interesting to consider using a TupleTableSlot instead of a bare
    HeapTuple for instance. In any case you're ignoring the point that
    other things will get faster.

    regards, tom lane
  • Pavel Stehule at Sep 21, 2012 at 4:21 am

    2012/9/20 Tom Lane <tgl@sss.pgh.pa.us>:
    Pavel Stehule <pavel.stehule@gmail.com> writes:
    2012/9/20 Tom Lane <tgl@sss.pgh.pa.us>:
    I'm not sure what the performance tradeoffs would be ---
    some things would get faster and others slower, probably, since field
    access would be more work but conversion to/from HeapTuple would get far
    cheaper.
    when fields are fix length, then field's update is significantly
    faster then when RECORD is used
    [ shrug... ] Maybe not if we put a little effort into it. It would be
    interesting to consider using a TupleTableSlot instead of a bare
    HeapTuple for instance. In any case you're ignoring the point that
    other things will get faster.
    I didn't try to optimize this. But these tests showed important impact.

    postgres=# create table foo(a int, b int, c int);
    CREATE TABLE
    postgres=# insert into foo select 1,2,3 from generate_series(1,100000);
    INSERT 0 100000
    postgres=# select count(*) from foo;
    count
    --------
    100000
    (1 row)

    postgres=# do $$ declare r record; begin for r in select * from foo
    loop perform r.a + r.b + r.c; end loop; end; $$ language plpgsql;
    DO
    Time: 712.404 ms

    postgres=# do $$ declare r foo; begin for r in select * from foo loop
    perform r.a + r.b + r.c; end loop; end; $$ language plpgsql;
    DO
    Time: 1617.847 ms

    In this case, record is faster - it was used in PL/PSM0 too

    postgres=# do $$ declare r record; begin for r in select * from foo
    loop r.a := r.b + r.c; end loop; end; $$ language plpgsql;
    DO
    Time: 170.701 ms

    postgres=# do $$ declare r foo; begin for r in select * from foo loop
    r.a := r.b + r.c; end loop; end; $$ language plpgsql;
    DO
    Time: 96.467 ms

    or

    postgres=# do $$ declare r record; t int; begin for r in select * from
    foo loop t := r.b + r.c; end loop; end; $$ language plpgsql;
    DO
    Time: 127.760 ms

    postgres=# do $$ declare r foo; t int; begin for r in select * from
    foo loop t := r.b + r.c; end loop; end; $$ language plpgsql;
    DO
    Time: 87.003 ms

    typed composite variable is significantly faster in simple queries (expressions)

    But I invite any reduction in this area

    regards, Pavel
    regards, tom lane

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedSep 20, '12 at 9:05p
activeSep 21, '12 at 4:21a
posts5
users3
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2021 Grokbase