From: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
---|---|
To: | Tom Kazimiers <tom(at)voodoo-arts(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | PG-General Mailing List <pgsql-general(at)postgresql(dot)org> |
Subject: | Re: Unexpected behavior with transition tables in update statement trigger |
Date: | 2018-02-27 01:52:02 |
Message-ID: | CAEepm=2PwOduZY+2CpO=2Z4UYQANKA_j4NqJiinH6B4kaunYCw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-general pgsql-hackers |
On Tue, Feb 27, 2018 at 4:18 AM, Tom Kazimiers <tom(at)voodoo-arts(dot)net> wrote:
> On Mon, Feb 26, 2018 at 11:15:44PM +1300, Thomas Munro wrote:
>> On Sat, Feb 24, 2018 at 4:47 PM, Tom Kazimiers <tom(at)voodoo-arts(dot)net>
>> wrote:
>> Thanks for the reproducer. Yeah, that seems to be a bug.
>> nodeNamedTuplestorescan.c allocates a new read pointer for each
>> separate scan of the named tuplestore, but it doesn't call
>> tuplestore_select_read_pointer() so that the two scans that appear in
>> your UNION ALL plan are sharing the same read pointer. At first
>> glance the attached seems to fix the problem, but I'll need to look
>> more carefully tomorrow.
>
> Thanks very much for investigating this. I can confirm that applying your
> patch results in the tuples I expected in both my test trigger and my actual
> trigger function.
Thanks for testing.
> It would be great if this or a similar fix would make it into the next
> official release.
Here's a new version with tuplestore_select_read_pointer() added in
another place where it was lacking, and commit message. Moving to
-hackers, where patches go.
Here's a shorter repro. On master it prints:
NOTICE: count = 1
NOTICE: count union = 1
With the patch the second number is 2, as it should be.
CREATE TABLE test (i int);
INSERT INTO test VALUES (1);
CREATE OR REPLACE FUNCTION my_trigger_fun() RETURNS trigger
LANGUAGE plpgsql AS
$$
BEGIN
RAISE NOTICE 'count = %', (SELECT COUNT(*) FROM new_test);
RAISE NOTICE 'count union = %', (SELECT COUNT(*)
FROM (SELECT * FROM new_test UNION ALL SELECT * FROM new_test) ss);
RETURN NULL;
END;
$$;
CREATE TRIGGER my_trigger AFTER UPDATE ON test
REFERENCING NEW TABLE AS new_test OLD TABLE as old_test
FOR EACH STATEMENT EXECUTE PROCEDURE my_trigger_fun();
UPDATE test SET i = i;
--
Thomas Munro
http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
0001-Fix-tuplestore-read-pointer-confusion-in-nodeNamedtu.patch | application/octet-stream | 1.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Melvin Davidson | 2018-02-27 02:24:14 | Re: system catalog permissions |
Previous Message | David G. Johnston | 2018-02-27 00:50:56 | Re: system catalog permissions |
From | Date | Subject | |
---|---|---|---|
Next Message | Rushabh Lathia | 2018-02-27 01:57:45 | Re: invalid memory alloc request size error with commit 4b93f579 |
Previous Message | Tatsuo Ishii | 2018-02-27 01:39:57 | Re: TODO item for broken \s with libedit seems fixed |