Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

Kristjan Tammekivi
Hi,

I've noticed a change in the behaviour in triggers / hstores in Postgres 11.1 when compared to Postgres 10.5.
The following won't work on Postgres 10.5 but in Postgres 11.1 it works just fine:

CREATE EXTENSION hstore;

CREATE TABLE _tmp_test1 (id serial PRIMARY KEY, val INTEGER);
CREATE TABLE _tmp_test1_changes (id INTEGER, changes HSTORE);

CREATE FUNCTION test1_trigger ()
RETURNS TRIGGER
LANGUAGE plpgsql
AS
$BODY$
BEGIN
INSERT INTO _tmp_test1_changes (id, changes) VALUES (NEW.id, hstore(OLD) - hstore(NEW));
RETURN NEW;
END
$BODY$;

CREATE TRIGGER table_update AFTER INSERT OR UPDATE ON _tmp_test1
FOR EACH ROW EXECUTE PROCEDURE test1_trigger();

INSERT INTO _tmp_test1 (val) VALUES (5);
ERROR:  record "old" is not assigned yet
DETAIL:  The tuple structure of a not-yet-assigned record is indeterminate.
CONTEXT:  SQL statement "INSERT INTO _tmp_test1_changes (id, changes) VALUES (NEW.id, hstore(OLD) - hstore(NEW))"
PL/pgSQL function test1_trigger() line 3 at SQL statement

I couldn't find anything about this in the release notes (https://www.postgresql.org/docs/11/release-11.html), but maybe I just didn't know what to look for.
Reply | Threaded
Open this post in threaded view
|

RE: Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

Charles Clavadetscher

Hello

 

From: Kristjan Tammekivi <[hidden email]>
Sent: Freitag, 4. Januar 2019 11:46
To: [hidden email]
Subject: Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

 

Hi,

 

I've noticed a change in the behaviour in triggers / hstores in Postgres 11.1 when compared to Postgres 10.5.

The following won't work on Postgres 10.5 but in Postgres 11.1 it works just fine:

 

CREATE EXTENSION hstore;

CREATE TABLE _tmp_test1 (id serial PRIMARY KEY, val INTEGER);
CREATE TABLE _tmp_test1_changes (id INTEGER, changes HSTORE);

CREATE FUNCTION test1_trigger ()
RETURNS TRIGGER
LANGUAGE plpgsql
AS
$BODY$
BEGIN
INSERT INTO _tmp_test1_changes (id, changes) VALUES (NEW.id, hstore(OLD) - hstore(NEW));
RETURN NEW;
END
$BODY$;

CREATE TRIGGER table_update AFTER INSERT OR UPDATE ON _tmp_test1
FOR EACH ROW EXECUTE PROCEDURE test1_trigger();

 

INSERT INTO _tmp_test1 (val) VALUES (5);

ERROR:  record "old" is not assigned yet

DETAIL:  The tuple structure of a not-yet-assigned record is indeterminate.

CONTEXT:  SQL statement "INSERT INTO _tmp_test1_changes (id, changes) VALUES (NEW.id, hstore(OLD) - hstore(NEW))"

PL/pgSQL function test1_trigger() line 3 at SQL statement

 

I couldn't find anything about this in the release notes (https://www.postgresql.org/docs/11/release-11.html), but maybe I just didn't know what to look for.

 

I doubt that this works on any PG version for INSERT.

 

According to the documentation:

 

https://www.postgresql.org/docs/10/plpgsql-trigger.html and https://www.postgresql.org/docs/11/plpgsql-trigger.html

 

OLD: Data type RECORD; variable holding the old database row for UPDATE/DELETE operations in row-level triggers. This variable is unassigned in statement-level triggers and for INSERT operations.

 

Regards

Charles

Reply | Threaded
Open this post in threaded view
|

Re: Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

Kristjan Tammekivi
Hi,
I've read the documentation, that's why I said this might be undocumented. Try the SQL in Postgres 11 and see that it works for yourself.
I have an analogous trigger in production from yesterday and I've tested it in local environment as well.

On Fri, Jan 4, 2019 at 12:56 PM Charles Clavadetscher <[hidden email]> wrote:

Hello

 

From: Kristjan Tammekivi <[hidden email]>
Sent: Freitag, 4. Januar 2019 11:46
To: [hidden email]
Subject: Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

 

Hi,

 

I've noticed a change in the behaviour in triggers / hstores in Postgres 11.1 when compared to Postgres 10.5.

The following won't work on Postgres 10.5 but in Postgres 11.1 it works just fine:

 

CREATE EXTENSION hstore;

CREATE TABLE _tmp_test1 (id serial PRIMARY KEY, val INTEGER);
CREATE TABLE _tmp_test1_changes (id INTEGER, changes HSTORE);

CREATE FUNCTION test1_trigger ()
RETURNS TRIGGER
LANGUAGE plpgsql
AS
$BODY$
BEGIN
INSERT INTO _tmp_test1_changes (id, changes) VALUES (NEW.id, hstore(OLD) - hstore(NEW));
RETURN NEW;
END
$BODY$;

CREATE TRIGGER table_update AFTER INSERT OR UPDATE ON _tmp_test1
FOR EACH ROW EXECUTE PROCEDURE test1_trigger();

 

INSERT INTO _tmp_test1 (val) VALUES (5);

ERROR:  record "old" is not assigned yet

DETAIL:  The tuple structure of a not-yet-assigned record is indeterminate.

CONTEXT:  SQL statement "INSERT INTO _tmp_test1_changes (id, changes) VALUES (NEW.id, hstore(OLD) - hstore(NEW))"

PL/pgSQL function test1_trigger() line 3 at SQL statement

 

I couldn't find anything about this in the release notes (https://www.postgresql.org/docs/11/release-11.html), but maybe I just didn't know what to look for.

 

I doubt that this works on any PG version for INSERT.

 

According to the documentation:

 

https://www.postgresql.org/docs/10/plpgsql-trigger.html and https://www.postgresql.org/docs/11/plpgsql-trigger.html

 

OLD: Data type RECORD; variable holding the old database row for UPDATE/DELETE operations in row-level triggers. This variable is unassigned in statement-level triggers and for INSERT operations.

 

Regards

Charles

Reply | Threaded
Open this post in threaded view
|

Re: Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

Adrian Klaver-4
On 1/4/19 4:20 AM, Kristjan Tammekivi wrote:
> Hi,
> I've read the documentation, that's why I said this might be
> undocumented. Try the SQL in Postgres 11 and see that it works for yourself.
> I have an analogous trigger in production from yesterday and I've tested
> it in local environment as well.

I can confirm:

select version();
                                       version

------------------------------------------------------------------------------------
  PostgreSQL 11.1 on x86_64-pc-linux-gnu, compiled by gcc (SUSE Linux)
4.8.5, 64-bit


INSERT INTO _tmp_test1 (val) VALUES (5);
INSERT 0 1

select * from _tmp_test1_changes ;
  id |         changes
----+-------------------------
   1 | "id"=>NULL, "val"=>NULL
(1 row)

I would file a bug report:

https://www.postgresql.org/account/submitbug/

>
> On Fri, Jan 4, 2019 at 12:56 PM Charles Clavadetscher
> <[hidden email] <mailto:[hidden email]>> wrote:
>
>     Hello____
>
>     __ __
>
>     *From:*Kristjan Tammekivi <[hidden email]
>     <mailto:[hidden email]>>
>     *Sent:* Freitag, 4. Januar 2019 11:46
>     *To:* [hidden email] <mailto:[hidden email]>
>     *Subject:* Potentially undocumented behaviour change in Postgres 11
>     concerning OLD record in an after insert trigger____
>
>     __ __
>
>     Hi,____
>
>     __ __
>
>     I've noticed a change in the behaviour in triggers / hstores in
>     Postgres 11.1 when compared to Postgres 10.5.____
>
>     The following won't work on Postgres 10.5 but in Postgres 11.1 it
>     works just fine:____
>
>     __ __
>
>     CREATE EXTENSION hstore;
>
>     CREATE TABLE _tmp_test1 (id serial PRIMARY KEY, val INTEGER);
>     CREATE TABLE _tmp_test1_changes (id INTEGER, changes HSTORE);
>
>     CREATE FUNCTION test1_trigger ()
>     RETURNS TRIGGER
>     LANGUAGE plpgsql
>     AS
>     $BODY$
>     BEGIN
>     INSERT INTO _tmp_test1_changes (id, changes) VALUES (NEW.id,
>     hstore(OLD) - hstore(NEW));
>     RETURN NEW;
>     END
>     $BODY$;
>
>     CREATE TRIGGER table_update AFTER INSERT OR UPDATE ON _tmp_test1
>     FOR EACH ROW EXECUTE PROCEDURE test1_trigger();____
>
>     __ __
>
>     INSERT INTO _tmp_test1 (val) VALUES (5);____
>
>     ERROR:  record "old" is not assigned yet____
>
>     DETAIL:  The tuple structure of a not-yet-assigned record is
>     indeterminate.____
>
>     CONTEXT:  SQL statement "INSERT INTO _tmp_test1_changes (id,
>     changes) VALUES (NEW.id, hstore(OLD) - hstore(NEW))"____
>
>     PL/pgSQL function test1_trigger() line 3 at SQL statement____
>
>     __ __
>
>     I couldn't find anything about this in the release notes
>     (https://www.postgresql.org/docs/11/release-11.html), but maybe I
>     just didn't know what to look for.____
>
>     __ __
>
>     *I doubt that this works on any PG version for INSERT.____*
>
>     *__ __*
>
>     *According to the documentation:____*
>
>     *__ __*
>
>     *https://www.postgresql.org/docs/10/plpgsql-trigger.html and
>     https://www.postgresql.org/docs/11/plpgsql-trigger.html____*
>
>     *__ __*
>
>     *OLD: **Data type **RECORD**; variable holding the old database row
>     for **UPDATE**/**DELETE**operations in row-level triggers. This
>     variable is unassigned in statement-level triggers and for
>     **INSERT**operations.**____*
>
>     *__ __*
>
>     *Regards____*
>
>     *Charles____*
>


--
Adrian Klaver
[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

Tom Lane-2
In reply to this post by Kristjan Tammekivi
Kristjan Tammekivi <[hidden email]> writes:
> I've noticed a change in the behaviour in triggers / hstores in Postgres
> 11.1 when compared to Postgres 10.5.
> [ reference to OLD in an insert trigger doesn't throw error anymore ]

Hmm.  This seems to be a side effect of the changes we (I) made in v11
to rationalize the handling of NULL vs ROW(NULL,NULL,...) composite
values in plpgsql.  The "unassigned" trigger row variables are now
acting as though they are plain NULL values.  I'm not sure now whether
I realized that this would happen --- it looks like there are not and
were not any regression test cases that would throw an error for the
disallowed-reference case, so it's quite possible that it just escaped
notice.

The old behavior was pretty darn squirrely; in particular it would let
you parse but not execute a reference to "OLD.column", a behavior that
could not occur for any other composite variable.  Now that'll just
return NULL.  In a green field I don't think there'd be complaints
about this behavior --- I know lots of people have spent considerable
effort programming around the other behavior.

While I haven't looked closely, I think duplicating the old behavior
would require adding a special-purpose flag to plpgsql DTYPE_REC
variables, which'd cost a little performance (extra error checks
in very hot code paths) and possibly break compatibility with
pldebugger, if there's been a v11 release of that.

So I'm a bit inclined to accept this behavior change and adjust
the documentation to say that OLD/NEW are "null" rather than
"unassigned" when not relevant.

Thoughts?

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: Potentially undocumented behaviour change in Postgres 11 concerning OLD record in an after insert trigger

Pavel Stehule


pá 4. 1. 2019 v 17:44 odesílatel Tom Lane <[hidden email]> napsal:
Kristjan Tammekivi <[hidden email]> writes:
> I've noticed a change in the behaviour in triggers / hstores in Postgres
> 11.1 when compared to Postgres 10.5.
> [ reference to OLD in an insert trigger doesn't throw error anymore ]

Hmm.  This seems to be a side effect of the changes we (I) made in v11
to rationalize the handling of NULL vs ROW(NULL,NULL,...) composite
values in plpgsql.  The "unassigned" trigger row variables are now
acting as though they are plain NULL values.  I'm not sure now whether
I realized that this would happen --- it looks like there are not and
were not any regression test cases that would throw an error for the
disallowed-reference case, so it's quite possible that it just escaped
notice.

The old behavior was pretty darn squirrely; in particular it would let
you parse but not execute a reference to "OLD.column", a behavior that
could not occur for any other composite variable.  Now that'll just
return NULL.  In a green field I don't think there'd be complaints
about this behavior --- I know lots of people have spent considerable
effort programming around the other behavior.

While I haven't looked closely, I think duplicating the old behavior
would require adding a special-purpose flag to plpgsql DTYPE_REC
variables, which'd cost a little performance (extra error checks
in very hot code paths) and possibly break compatibility with
pldebugger, if there's been a v11 release of that.

So I'm a bit inclined to accept this behavior change and adjust
the documentation to say that OLD/NEW are "null" rather than
"unassigned" when not relevant.

It is maybe unwanted effect, but it is not bad from my view. new behave is consistent - a initial value of variables is just NULL

+1

Pavel


Thoughts?

                        regards, tom lane