MIME-Version: 1.0
Date: Thu, 22 Jun 2023 14:07:42 +0300
From: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: memory leak in trigger handling (since PG12)
In-Reply-To: <b3c8da23-8850-7697-2aaf-947bb2a4d0a3@enterprisedb.com>
References: <222a3442-7f7d-246c-ed9b-a76209d19239@enterprisedb.com>
 <960113.1684859980@sss.pgh.pa.us>
 <a5d42a2c-0c2e-9fb8-793f-0675d85669be@enterprisedb.com>
 <f706d912-b6fa-f1d7-5a39-dc1be5999183@enterprisedb.com>
 <20230524185559.un4dqfem3u3ktons@awork3.anarazel.de>
 <c3ff31ce-622c-81a9-9dcc-dbb27c5eaf9e@enterprisedb.com>
 <b3c8da23-8850-7697-2aaf-947bb2a4d0a3@enterprisedb.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <acddb17c89b0d6cb940eaeda18c08bbe@postgrespro.ru>
Content-Type: multipart/mixed;
 boundary="=_1a937a31e9443ace6a2a0771822b79f2"
Archived-At: 
 <https://www.postgresql.org/message-id/acddb17c89b0d6cb940eaeda18c08bbe%40postgrespro.ru>
Precedence: bulk

--=_1a937a31e9443ace6a2a0771822b79f2
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=UTF-8;
 format=flowed

Tomas Vondra писал 2023-05-25 17:41:

> The attached patch does this - I realized we actually have estate in
> ExecGetAllUpdatedCols(), so we don't even need a variant with a
> different signature. That makes the patch much simpler.
> 
> The question is whether we need the signature anyway. There might be a
> caller expecting the result to be in CurrentMemoryContext (be it
> ExecutorState or something else), and this change might break it. But
> I'm not aware of any callers, so maybe that's fine.
> 

Hi.
Unfortunately, this patch has broken trigdata->tg_updatedcols usage in 
AFTER UPDATE triggers.
Should it be if not fixed, but at least mentioned in documentation?

Attaching sample code. After creating trigger, an issue can be 
reproduced as this:

create table test (i int, j int);
create function report_update_fields() RETURNS TRIGGER AS 
'/location/to/trig_test.so' language C;
create trigger test_update after update ON test FOR EACH ROW EXECUTE 
FUNCTION report_update_fields();
insert into test values (1, 12);
update test set j=2;

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional
--=_1a937a31e9443ace6a2a0771822b79f2
Content-Transfer-Encoding: base64
Content-Type: text/x-c;
 name=trig_test.c
Content-Disposition: attachment;
 filename=trig_test.c;
 size=1116

I2luY2x1ZGUgPHBvc3RncmVzLmg+CiNpbmNsdWRlIDxmbWdyLmg+CiNpbmNsdWRlIDxmdW5jYXBp
Lmg+CiNpbmNsdWRlIDx1dGlscy9lbG9nLmg+CiNpbmNsdWRlIDxjb21tYW5kcy90cmlnZ2VyLmg+
CgojaWZkZWYgUEdfTU9EVUxFX01BR0lDClBHX01PRFVMRV9NQUdJQzsKI2VuZGlmCgoKdm9pZCBf
UEdfaW5pdCgpCnsKfQoKUEdfRlVOQ1RJT05fSU5GT19WMShyZXBvcnRfdXBkYXRlX2ZpZWxkcyk7
CgpEYXR1bQpyZXBvcnRfdXBkYXRlX2ZpZWxkcyhQR19GVU5DVElPTl9BUkdTKQp7CglUcmlnZ2Vy
RGF0YSAqdHJpZ2RhdGEgPSAoVHJpZ2dlckRhdGEgKikgZmNpbmZvLT5jb250ZXh0OwoJaW50CQkJ
dGduYXJncywKCQkJCXJldDsKCWNoYXIJICAgKnJnaWRzLAoJCQkgICAqZ3RuYW1lLAoJCQkgICAq
bnNwbmFtZSwKCQkJICAgKnJuYW1lOwoJSGVhcFR1cGxlCXJldHR1cGxlID0gTlVMTDsKCUxpc3QJ
ICAgKnJnaWRsaXN0ID0gTklMOwoJTGlzdENlbGwgICAqbGM7CgoJaWYgKCFDQUxMRURfQVNfVFJJ
R0dFUihmY2luZm8pIHx8IC8qIVRSSUdHRVJfRklSRURfQUZURVIodHJpZ2RhdGEtPnRnX2V2ZW50
KSB8fCAqLyBUUklHR0VSX0ZJUkVEX0ZPUl9TVEFURU1FTlQodHJpZ2RhdGEtPnRnX2V2ZW50KSB8
fCAhVFJJR0dFUl9GSVJFRF9CWV9VUERBVEUodHJpZ2RhdGEtPnRnX2V2ZW50KSkKCQllbG9nKEVS
Uk9SLCAidXNhZ2UgdW5zdXBwb3J0ZWQiKTsKCQoJaWYgKCh0cmlnZGF0YS0+dGdfdHJpZ3R1cGxl
ICE9IE5VTEwpICYmICh0cmlnZGF0YS0+dGdfbmV3dHVwbGUgIT0gTlVMTCkpCgl7CiAgICAgICAg
aW50IGF0dG51bSA9IC0xOwogICAgICAgIGludCBudW1fdXBkX2F0dHJzID0gMDsKICAgICAgICAK
ICAgICAgICB3aGlsZSAoKGF0dG51bSA9IGJtc19uZXh0X21lbWJlcih0cmlnZGF0YS0+dGdfdXBk
YXRlZGNvbHMsIGF0dG51bSkpID49IDApCiAgICAgICAgICAgIGlmIChhdHRudW0gKyBGaXJzdExv
d0ludmFsaWRIZWFwQXR0cmlidXRlTnVtYmVyID4gMCkKICAgICAgICAgICAgICAgIG51bV91cGRf
YXR0cnMrKzsKICAgICAgICAgICAgICAgIAogICAgICAgIGVsb2coSU5GTywgInVwZGF0ZWQgJWQg
YXR0cnMiLCBudW1fdXBkX2F0dHJzKTsKICAgIH0KfQoK
--=_1a937a31e9443ace6a2a0771822b79f2--