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

Hello

From: Kristjan Tammekivi <kristjantammekivi@gmail.com>
Sent: Freitag, 4. Januar 2019 11:46
To: pgsql-general@postgresql.org
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

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 <
clavadetscher@swisspug.org> wrote:

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
<clavadetscher@swisspug.org <mailto:clavadetscher@swisspug.org>> wrote:

Hello____

__ __

*From:*Kristjan Tammekivi <kristjantammekivi@gmail.com
<mailto:kristjantammekivi@gmail.com>>
*Sent:* Freitag, 4. Januar 2019 11:46
*To:* pgsql-general@postgresql.org <mailto:pgsql-general@postgresql.org>
*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
adrian.klaver@aklaver.com

Kristjan Tammekivi <kristjantammekivi@gmail.com> 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

pá 4. 1. 2019 v 17:44 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

Kristjan Tammekivi <kristjantammekivi@gmail.com> 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