Inconsistent trigger behavior between two temporal leftovers
Hi hackers,
The FOR PORTION OF thread [1]/messages/by-id/ec498c3d-5f2b-48ec-b989-5561c8aa2024@illuminatedcomputing.com noted the challenges in defining the
interaction between FPO and insert triggers.
Where an UPDATE FOR PORTION OF produces two temporal leftovers, I feel
trigger behavior should be
symmetrical between the two leftovers, whatever that behavior may be.
Currently a tuple-modifying BEFORE INSERT ROW trigger will fire for
both leftovers, but the second leftover's trigger
will see tuple modifications from the first leftover's trigger. I feel
this produces a surprising asymmetry:
```
CREATE TABLE products (id int, valid_at daterange, name text, revision int);
CREATE FUNCTION increment_product_revision() RETURNS trigger LANGUAGE
plpgsql AS $$
BEGIN NEW.revision := NEW.revision + 1; RETURN NEW; END; $$;
CREATE TRIGGER products_insert_trigger BEFORE INSERT ON products
FOR EACH ROW EXECUTE FUNCTION increment_product_revision();
INSERT INTO products VALUES (1, '[2020-01-01, 2020-12-31)', 'widget', 0);
-- Update producing two leftovers
UPDATE products FOR PORTION OF valid_at FROM '2020-04-01' TO '2020-08-01'
SET name = 'gadget' WHERE id = 1;
SELECT * FROM products ORDER BY valid_at;
-- id | valid_at | name | revision
-- ---+-------------------------+--------+---------
-- 1 | [2020-01-01,2020-04-01) | widget | 2
-- 1 | [2020-04-01,2020-08-01) | gadget | 1
-- 1 | [2020-08-01,2020-12-31) | widget | 3
-- first leftover has revision=2 - ok
-- second leftover has revision=3 - surprising?
```
[1]: /messages/by-id/ec498c3d-5f2b-48ec-b989-5561c8aa2024@illuminatedcomputing.com
Best regards,
Sergei
Attaching a patch that makes the behavior more consistent.
Best regards,
Sergei
Attachments:
0001-Consistent-trigger-behavior-between-multiple-tempora.patchapplication/octet-stream; name=0001-Consistent-trigger-behavior-between-multiple-tempora.patchDownload+87-1
On Wed, Apr 8, 2026 at 5:50 AM Sergei Patiakin
<sergei.patiakin@enterprisedb.com> wrote:
Attaching a patch that makes the behavior more consistent.
Thank you for the bug report!
I agree this seems surprising. The asymmetry doesn't bother me per se,
but each insert should have its own copy of the original row. It
doesn't make sense for a trigger firing on one leftover to affect the
other.
I wrote a fix before I saw yours, but they are nearly the same. I
tried to share more code between the first & subsequent passes, but
it's hard to do with the mapping case. So the fix you have here looks
great to me.
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com
On Tue, Apr 14, 2026 at 10:28 PM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:
I wrote a fix before I saw yours, but they are nearly the same. I
tried to share more code between the first & subsequent passes, but
it's hard to do with the mapping case. So the fix you have here looks
great to me.
I made a commitfest entry for this:
https://commitfest.postgresql.org/patch/6702/
I am currently listed as both an author and reviewer. Sergei, I'd like
to add you as the author, but I don't see an account with your name in
the commitfest app. If you have one, please feel free to add yourself
(or let me know which account I should use).
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com
On 08.04.26 14:50, Sergei Patiakin wrote:
Attaching a patch that makes the behavior more consistent.
Maybe this is not a problem in your patch, but I'm confused under what
circumstances one is supposed to use the return value of
execute_attr_map_slot(). Existing code appears to be inconsistent about
that, but there isn't any explanation anywhere I can see.
If you have one, please feel free to add yourself
Thanks Paul, I've done that now
Maybe this is not a problem in your patch, but I'm confused under what
circumstances one is supposed to use the return value of
execute_attr_map_slot(). Existing code appears to be inconsistent about
that, but there isn't any explanation anywhere I can see.
It looks like `execute_attr_map_slot(attrMap, in_slot, out_slot)` always
returns `out_slot`, so assigning the return value to `out_slot` is a no-op.
I see a couple of such no-op assignments in the codebase - we could remove
them if we think it improves clarity?
I don't have any objections to execute_attr_map_slot's interface - it
seems conventional for functions that copy/transform a data structure
into a caller-passed buffer to also return a pointer to the buffer,
e.g. ExecCopySlot, strcpy(3), realpath(3)
Show quoted text
On Mon, Apr 27, 2026 at 11:40 AM Peter Eisentraut <peter@eisentraut.org> wrote:
On 08.04.26 14:50, Sergei Patiakin wrote:
Attaching a patch that makes the behavior more consistent.
Maybe this is not a problem in your patch, but I'm confused under what
circumstances one is supposed to use the return value of
execute_attr_map_slot(). Existing code appears to be inconsistent about
that, but there isn't any explanation anywhere I can see.
Hello
+ /*
+ * Re-copy the original row into leftoverSlot because
+ * ExecInsert may pass leftoverSlot to BEFORE INSERT
+ * triggers, which can modify the slot contents.
+ */Shouldn't this mention BEFORE ROW INSERT triggers? Everywhere else the
comments/commit message/test is specific about this.
On Tue, May 5, 2026 at 2:16 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
+ /* + * Re-copy the original row into leftoverSlot because + * ExecInsert may pass leftoverSlot to BEFORE INSERT + * triggers, which can modify the slot contents. + */Shouldn't this mention BEFORE ROW INSERT triggers? Everywhere else the
comments/commit message/test is specific about this.
I agree being specific is helpful. Here is an updated patch.
Yours,
--
Paul ~{:-)
pj@illuminatedcomputing.com