Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi,

On Tue, Jul 1, 2025 at 5:47 PM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
>
> On Mon, 30 Jun 2025 18:32:47 +0700
> Daniil Davydov <3danissimo(at)gmail(dot)com> wrote:
>
> > On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> > >
> > > I agree with Tom Lane that the behavior is expected, although it would be better
> > > if the error message were more user-friendly. We could improve it by checking the
> > > unique constraint before calling index_insert in CatalogIndexInsert.
> > >
> >
> > As far as I understand, unique constraint checking is specific for
> > each index access method.
> > Thus, to implement the proposed idea, you will have to create a
> > separate callback for check_unique function.
> > It doesn't seem like a very neat solution, but there aren't many other
> > options left.
>
> I believe check_exclusion_or_unique_constraint() can be used independently of
> a specific index access method.
>
> > I would suggest intercepting the error (via PG_CATCH), and if it has
> > the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
> > precisely, throw another error with the desired message).
> > If we caught an error during the CatalogTupleInsert call, we can be
> > sure that the problem is in concurrent execution, because before the
> > insertion, we checked that such a tuple does not exist.
> >
> > What do you think? And in general, are you going to fix this behavior
> > within this thread?
>
> Initially, I wasn't planning to do so, but I gave it a try and wrote a
> patch to fix the issue based on my idea.

Thanks for the patch! Some comments on it :
1)
I found two typos :
+ if (HeapTupleIsHeapOenly(heapTuple) && !onlySummarized)
and
+ sartisfied = check_unique_constraint(heapRelation,

2)
CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true.

That is why I suggested handling unique violations exactly inside
ProcedureCreate - the only place where we can be sure about reasons of
error.

--
Best regards,
Daniil Davydov