Does 'instead of delete' trigger support modification of OLD

On Tue, Oct 29, 2019 at 05:54:36PM +0200, Eugen Konkov wrote:

Hi.

This is not clear from doc, so I have asked on IRC too.

from the DOC: https://www.postgresql.org/docs/current/trigger-definition.html
In the case of INSTEAD OF triggers, the possibly-modified row returned by each trigger becomes the input to the next trigger

I modify OLD row, thus I expect to get modified version when run next query:

WITH t1 AS( delete from abc returning *)
select * from t1;

fiddle: https://dbfiddle.uk/?rdbms=postgres_12&fiddle=637730305f66bf531794edb09a462c95

Wow, that is a very nice way to present the queries.

https://www.postgresql.org/docs/current/trigger-definition.html

A row-level INSTEAD OF trigger should either return NULL to indicate that it did not modify any data from the view's underlying base tables,
or it should return the view row that was passed in (the NEW row for INSERT and UPDATE operations, or the OLD row for DELETE operations).
A nonnull return value is used to signal that the trigger performed the necessary data modifications in the view.
This will cause the count of the number of rows affected by the command to be incremented. For INSERT and UPDATE operations, the trigger may
modify the NEW row before returning it. This will change the data returned by INSERT RETURNING or UPDATE RETURNING,
and is useful when the view will not show exactly the same data that was provided.

But I still does not understand. Doc explicitly do not prohibit modification of OLD and has no examples for DELETE RETURNING case

I looked in the CREATE TRIGGER manual page and found this:

https://www.postgresql.org/docs/12/sql-createtrigger.html
If the trigger fires before or instead of the event, the trigger
can skip the operation for the current row, or change the row
being inserted (for INSERT and UPDATE operations only).

I don't see the "(for INSERT and UPDATE operations only)" language in
the main trigger documentation,
https://www.postgresql.org/docs/current/trigger-definition.html. I have
written the attached patch to fix that. Does that help?

As far as allowing DELETE to modify the trigger row for RETURNING, I am
not sure how much work it would take to allow that, but it seems like it
is a valid requite, and if so, I can add it to the TODO list.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

I looked in the CREATE TRIGGER manual page and found this:

https://www.postgresql.org/docs/12/sql-createtrigger.html
If the trigger fires before or instead of the event, the trigger
can skip the operation for the current row, or change the row
being inserted (for INSERT and UPDATE operations only).

I don't see the "(for INSERT and UPDATE operations only)" language in
the main trigger documentation,
https://www.postgresql.org/docs/current/trigger-definition.html. I have
written the attached patch to fix that. Does that help?

No. If we document that PG does not allow to modify OLD at instead
of trigger, the we can not implement that. Probably we can put note
that "currently modification of the trigger row for RETURNING is not
implemented"

As far as allowing DELETE to modify the trigger row for RETURNING, I am
not sure how much work it would take to allow that, but it seems like it
is a valid requite, and if so, I can add it to the TODO list.

Yes, Add please into TODO the feature to "allowing DELETE to modify the trigger row
for RETURNING". Becuase, as I have described at first letter, without
this the RETURNING rows **does not correspond actually deleted data**

Thank you.

--
Best regards,
Eugen Konkov

Hello Eugen,

Thursday, November 7, 2019, 11:20:32 AM, you wrote:

I looked in the CREATE TRIGGER manual page and found this:

https://www.postgresql.org/docs/12/sql-createtrigger.html
If the trigger fires before or instead of the event, the trigger
can skip the operation for the current row, or change the row
being inserted (for INSERT and UPDATE operations only).

I don't see the "(for INSERT and UPDATE operations only)" language in
the main trigger documentation,
https://www.postgresql.org/docs/current/trigger-definition.html. I have
written the attached patch to fix that. Does that help?

No. If we document that PG does not allow to modify OLD at instead
of trigger, the we can not implement that. Probably we can put note
that "currently modification of the trigger row for RETURNING is not
implemented"

sorry, typo. Please read:
"currently modification of the trigger row for DELETE RETURNING is notimplemented"

As far as allowing DELETE to modify the trigger row for RETURNING, I am
not sure how much work it would take to allow that, but it seems like it
is a valid requite, and if so, I can add it to the TODO list.

Yes, Add please into TODO the feature to "allowing DELETE to modify the trigger row
for RETURNING". Becuase, as I have described at first letter, without
this the RETURNING rows **does not correspond actually deleted data**

Thank you.

--
Best regards,
Eugen Konkov

On Thu, Nov 7, 2019 at 11:24:29AM +0200, Eugen Konkov wrote:

Hello Eugen,

Thursday, November 7, 2019, 11:20:32 AM, you wrote:

I looked in the CREATE TRIGGER manual page and found this:

https://www.postgresql.org/docs/12/sql-createtrigger.html
If the trigger fires before or instead of the event, the trigger
can skip the operation for the current row, or change the row
being inserted (for INSERT and UPDATE operations only).

I don't see the "(for INSERT and UPDATE operations only)" language in
the main trigger documentation,
https://www.postgresql.org/docs/current/trigger-definition.html. I have
written the attached patch to fix that. Does that help?

No. If we document that PG does not allow to modify OLD at instead
of trigger, the we can not implement that. Probably we can put note
that "currently modification of the trigger row for RETURNING is not
implemented"

sorry, typo. Please read:
"currently modification of the trigger row for DELETE RETURNING is notimplemented"

In looking at the existing docs, the bullet above the quoted text says:

For row-level INSERT and UPDATE triggers only, the returned row becomes
----
the row that will be inserted or will replace the row being updated.
This allows the trigger function to modify the row being inserted or
updated.

First, notice "only", which was missing from the later sentence:

For <command>INSERT</command> and <command>UPDATE</command>
operations [only], the trigger may modify the
<varname>NEW</varname> row before returning it.

which I have now added with my applied patch to all supported releases.

The major use of modifying NEW is to modify the data that goes into the
database, and its use to modify data seen by later executed triggers, or
by RETURNING, is only a side-effect of its primary purpose. Therefore,
it is not surprising that, since DELETE does not modify any data, just
removes it, that the modification of OLD to appear in later triggers or
RETURNING is not supported.

As far as allowing DELETE to modify the trigger row for RETURNING, I am
not sure how much work it would take to allow that, but it seems like it
is a valid requite, and if so, I can add it to the TODO list.

Yes, Add please into TODO the feature to "allowing DELETE to modify the trigger row
for RETURNING". Becuase, as I have described at first letter, without
this the RETURNING rows **does not correspond actually deleted data**

Thank you.

I have added a TODO item:

Allow DELETE triggers to modify rows, for use by RETURNING

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

On Thu, Nov 7, 2019 at 04:26:55PM -0500, Bruce Momjian wrote:

On Thu, Nov 7, 2019 at 11:24:29AM +0200, Eugen Konkov wrote:

As far as allowing DELETE to modify the trigger row for RETURNING, I am
not sure how much work it would take to allow that, but it seems like it
is a valid requite, and if so, I can add it to the TODO list.

Yes, Add please into TODO the feature to "allowing DELETE to modify the trigger row
for RETURNING". Becuase, as I have described at first letter, without
this the RETURNING rows **does not correspond actually deleted data**

Thank you.

I have added a TODO item:

Allow DELETE triggers to modify rows, for use by RETURNING

Thinking some more on this, I now don't think a TODO makes sense, so I
have removed it.

Triggers are designed to check and modify input data, and since DELETE
has no input data, it makes no sense. In the attached SQL script, you
can see that only the BEFORE INSERT trigger fires, so there is no way
even with INSERT to change what is passed after the write to RETURNING.
What you can do is to modify the returning expression, which is what I
have done for the last query --- hopefully that will help you.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Hello Bruce,

Friday, November 8, 2019, 12:28:18 AM, you wrote:

On Thu, Nov 7, 2019 at 04:26:55PM -0500, Bruce Momjian wrote:

On Thu, Nov 7, 2019 at 11:24:29AM +0200, Eugen Konkov wrote:

As far as allowing DELETE to modify the trigger row for RETURNING, I am
not sure how much work it would take to allow that, but it seems like it
is a valid requite, and if so, I can add it to the TODO list.

Yes, Add please into TODO the feature to "allowing DELETE to modify the trigger row
for RETURNING". Becuase, as I have described at first letter, without
this the RETURNING rows **does not correspond actually deleted data**

Thank you.

I have added a TODO item:

Allow DELETE triggers to modify rows, for use by RETURNING

Thinking some more on this, I now don't think a TODO makes sense, so I
have removed it.

Triggers are designed to check and modify input data, and since DELETE
has no input data, it makes no sense. In the attached SQL script, you
can see that only the BEFORE INSERT trigger fires, so there is no way
even with INSERT to change what is passed after the write to RETURNING.
What you can do is to modify the returning expression, which is what I
have done for the last query --- hopefully that will help you.

You lost my idea. First of all I am talking about views and an
INSTEAD OF triggers.

INSERT/UPDATE operation present which data is added into DB
DELETE operation present which data is deleted from DB
(in my case I am not deleted exact that data which matched by where.
See example below)

Thus INSTEAD OF INSERT/UPDATE triggers are designed to check and modify input data
eg. we can insert/update something different then incoming data (here
we are modifying NEW)

Thus INSTEAD OF DELETE triggers are designed to check and delete **output** data
eg. we can delete something different then underlaid data (here we are
modifying OLD)

for example, we have next data: 1 2 3 4 5 6 7 8
it is not presented by eight rows, but instead it is presented as one
row with range data type: [1..8]

When we insert data we will not get new row, we change current:
insert into table values ( 9 ) will result
[1..9]
instead of
[1..8]
9

So lets look into INSTEAD OF DELETE trigger when we deleting
data:
delete from table where x in ( 5, 6, 7 );
after deleting this we should get:
[1..4]
[8..9]

thus
with t1 as ( delete from table where x in ( 5, 6, 7 ) returning * )
select * from t1
should return:
[5..7]
instead of
[1..9]
because we does not delete ALL [1..9], we just delete ONLY [5..7]

Thus I need to change matched row OLD.x from [1..9] to [5..7]

Please reread my first letter. There I describe more real life example
when I am manipulating bi-temporal data.

where some value exist at given period:
id | app_period | value
7 [2019-01-01, 2019-04-05) 207

And I am deleting third month: [ 2019-03-01, 2019-04-01 )
with t1 as ( delete from table where app_period && [ 2019-03-01,
2019-04-01 ) returning * )
select * from t1;
7 [ 2019-03-01, 2019-04-01 ) 207

select * from table;
7 [ 2019-01-01, 2019-03-01 ) 207
7 [ 2019-04-01, 2019-04-05 ) 207

--
Best regards,
Eugen Konkov

Hello Eugen,

Saturday, November 9, 2019, 2:05:02 PM, you wrote:

Hello Bruce,

Friday, November 8, 2019, 12:28:18 AM, you wrote:

On Thu, Nov 7, 2019 at 04:26:55PM -0500, Bruce Momjian wrote:

On Thu, Nov 7, 2019 at 11:24:29AM +0200, Eugen Konkov wrote:

As far as allowing DELETE to modify the trigger row for RETURNING, I am
not sure how much work it would take to allow that, but it seems like it
is a valid requite, and if so, I can add it to the TODO list.

Yes, Add please into TODO the feature to "allowing DELETE to modify the trigger row
for RETURNING". Becuase, as I have described at first letter, without
this the RETURNING rows **does not correspond actually deleted data**

Thank you.

I have added a TODO item:

Allow DELETE triggers to modify rows, for use by RETURNING

Thinking some more on this, I now don't think a TODO makes sense, so I
have removed it.

Triggers are designed to check and modify input data, and since DELETE
has no input data, it makes no sense. In the attached SQL script, you
can see that only the BEFORE INSERT trigger fires, so there is no way
even with INSERT to change what is passed after the write to RETURNING.
What you can do is to modify the returning expression, which is what I
have done for the last query --- hopefully that will help you.

You lost my idea. First of all I am talking about views and an
INSTEAD OF triggers.

INSERT/UPDATE operation present which data is added into DB
DELETE operation present which data is deleted from DB
(in my case I am not deleted exact that data which matched by where.
See example below)

Thus INSTEAD OF INSERT/UPDATE triggers are designed to check and modify input data
eg. we can insert/update something different then incoming data (here
we are modifying NEW)

Thus INSTEAD OF DELETE triggers are designed to check and delete **output** data
eg. we can delete something different then underlaid data (here we are
modifying OLD)

for example, we have next data: 1 2 3 4 5 6 7 8
it is not presented by eight rows, but instead it is presented as one
row with range data type: [1..8]

When we insert data we will not get new row, we change current:
insert into table values ( 9 ) will result
[1..9]
instead of
[1..8]
9

So lets look into INSTEAD OF DELETE trigger when we deleting
data:
delete from table where x in ( 5, 6, 7 );
after deleting this we should get:
[1..4]
[8..9]

thus
with t1 as ( delete from table where x in ( 5, 6, 7 ) returning * )
select * from t1
should return:
[5..7]
instead of
[1..9]
because we does not delete ALL [1..9], we just delete ONLY [5..7]

Thus I need to change matched row OLD.x from [1..9] to [5..7]

Please reread my first letter. There I describe more real life example
when I am manipulating bi-temporal data.

where some value exist at given period:
id | app_period | value
7 [2019-01-01, 2019-04-05) 207

And I am deleting third month: [ 2019-03-01, 2019-04-01 )
with t1 as ( delete from table where app_period && [ 2019-03-01,
2019-04-01 ) returning * )
select * from t1;
7 [ 2019-03-01, 2019-04-01 ) 207

select * from table;
7 [ 2019-01-01, 2019-03-01 ) 207
7 [ 2019-04-01, 2019-04-05 ) 207

Here when data is deleted the next row is matched:
7 [2019-01-01, 2019-04-05) 207
and assigned to OLD.
Because I am deleting data ONLY from [ 2019-03-01, 2019-04-01 ) period
I am required to change OLD:

OLD.app_period = [ 2019-03-01, 2019-04-01 )

So I should get:

7 [ 2019-03-01, 2019-04-01 ) 207

instead of

7 [2019-01-01, 2019-04-05) 207

--
Best regards,
Eugen Konkov

8 нояб. 2019 г., в 0:26, Bruce Momjian <bruce@momjian.us> написал(а):

First, notice "only", which was missing from the later sentence:

For <command>INSERT</command> and <command>UPDATE</command>
operations [only], the trigger may modify the
<varname>NEW</varname> row before returning it.

which I have now added with my applied patch to all supported releases.

Hi Bruce,

I happened to browse recent documentation-related commits and I didn’t see this patch in REL_12_STABLE. Judging by the commit message, it should be applied there too.

On Mon, Nov 11, 2019 at 07:00:22PM +0300, Liudmila Mantrova wrote:

8 нояб. 2019 г., в 0:26, Bruce Momjian <bruce@momjian.us> написал(а):

First, notice "only", which was missing from the later sentence:

For <command>INSERT</command> and <command>UPDATE</command>
operations [only], the trigger may modify the
<varname>NEW</varname> row before returning it.

which I have now added with my applied patch to all supported releases.

Hi Bruce,

I happened to browse recent documentation-related commits and I didn’t see this patch in REL_12_STABLE. Judging by the commit message, it should be applied there too.

Wow, not sure how that happened, fixed.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Hi again.

Thinking some more on this, I now don't think a TODO makes sense, so I
have removed it.

Please look into this example: https://dbfiddle.uk/?rdbms=postgres_12&amp;fiddle=95ed9fab6870d7c4b6266ea4d93def13
This is real life code from our production.

You can see that this is important to get correct info about deleted
data

-- EXPECTED app_period: ["2018-08-20", "2018-08-25")
-- ACTUAL app_period: ["2018-08-14", )

Triggers are designed to check and modify input data, and since DELETE
has no input data, it makes no sense.

Please put back into TODO list this feature request to allow
triggers to modify output data.

INPUT -- receives data OK (behavior is expected)
UPDATE -- receives and returns data OK (behavior is expected)
DELETE -- returns data FAIL (behavior is not expected)

This is inconsistent to allow modify output data for UPDATE and
restrict to do this for DELETE

Thank you

--
Best regards,
Eugen Konkov

Hello Eugen,

https://dbfiddle.uk/?rdbms=postgres_12&amp;fiddle=95ed9fab6870d7c4b6266ea4d93def13

sorry, forget to update link to the latest example:
https://dbfiddle.uk/?rdbms=postgres_12&amp;fiddle=8e114ccc9f15a30ca3115cdc6c70d247

--
Best regards,
Eugen Konkov

Hello Bruce,

Triggers are designed to check and modify input data, and since DELETE
has no input data, it makes no sense.

Sorry, I am still ambiguous. You say that DELETE has no input data,
but doc says that it has:

https://www.postgresql.org/docs/current/trigger-definition.html
For a row-level trigger, the input data also includes ... the OLD row for ... DELETE triggers

Also restricting DELETE to change the returned data by DELETE
RETURNING seems as incomplete.

For example if triggers implement some compression.
-- insert into field ZZZZZ value
-- compress and actually store Zx5 into field
-- Delete this insert row
-- So user should get back that the value ZZZZZ was deleted and not Zx5.
Correct?

but currently user will see Zx5, because next code:

OLD.value = uncompress( OLD.value );

does not effect RETURNING =(

--
Best regards,
Eugen Konkov