Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

From: Daniil Davydov <3danissimo(at)gmail(dot)com>
To: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
Cc: Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Lakhin <exclusion(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>
Subject: Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION
Date: 2025-06-27 11:53:02
Message-ID: CAJDiXgjj=NnTssXiMZmhCmrieJsATjvLTp1=x8OVobf_2UH9Xg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Thu, Jun 5, 2025 at 5:21 PM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
>
> I've attached updated patches.
>

I have some comments on v4-0001 patch :
1)
heap_freetuple should be called for every tuple that we get from
SearchSysCacheCopy3.
But if tuple is valid after the first SearchSysCacheCopy3, we
overwrite the old pointer (by the second SearchSysCacheCopy3 call) and
forget to free it.
I suggest adding heap_freetuple call before the second SearchSysCacheCopy3 call.

2)
+ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+ Datum proargnames;
+ bool isnull;
+ const char *dropcmd;
Strange alignment. I guess you should keep the same alignment as in
deleted declarations.

3)
This patch fixes postgres behavior if I first create a function and
then try to CREATE OR REPLACE it in concurrent transactions.
But if the function doesn't exist and I try to call CREATE OR REPLACE
in concurrent transactions, I will get an error.
I wrote about it in this thread [1] and Tom Lane said that this
behavior is kinda expected.
Just in case, I decided to mention it here anyway - perhaps you will
have other thoughts on this matter.

[1] https://www.postgresql.org/message-id/flat/CAJDiXghv2JF5zbLyyybokWKM%2B-GYsTG%2Bhw7xseLNgJOJwf0%2B8w%40mail.gmail.com

--
Best regards,
Daniil Davydov

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-06-27 12:03:12 Re: [PATCH] Use DatumGetUInt32() for dsm_attach() in test_shm_mq_main()
Previous Message Matthias van de Meent 2025-06-27 11:34:35 vacuumlazy: Modernize count_nondeletable_pages