Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

[jian he]

On Tue, May 27, 2025 at 1:35 AM Yugo Nagata <nag...@sraoss.co.jp> wrote:
>
> > + /* Lock the function so nobody else can do anything with it. */
> > + LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, 
> > AccessExclusiveLock);
> > +
> > + /*
> > + * It is possible that by the time we acquire the lock on function,
> > + * concurrent DDL has removed it. We can test this by checking the
> > + * existence of function. We get the tuple again to avoid the risk
> > + * of function definition getting changed.
> > + */
> > + oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
> > + PointerGetDatum(procedureName),
> > + PointerGetDatum(parameterTypes),
> > + ObjectIdGetDatum(procNamespace));
> >
> > we already called LockDatabaseObject, concurrent DDL can
> > not do DROP FUNCTION or ALTER FUNCTION.
> > so no need to call SearchSysCacheCopy3 again?
>
> The function may be dropped *before* we call LockDatabaseObject.
> SearchSysCacheCopy3 is called for check this.
> Plese see AlterPublication() as a similar code example.
>

I am wondering, can we do it the following way for v2-0001?


    /* Check for pre-existing definition */
    oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
                                 PointerGetDatum(procedureName),
                                 PointerGetDatum(parameterTypes),
                                 ObjectIdGetDatum(procNamespace));
    if (HeapTupleIsValid(oldtup))
    {
        /* There is one; okay to replace it? */
        Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
        if (!replace)
            ereport(ERROR,
                    (errcode(ERRCODE_DUPLICATE_FUNCTION),
                     errmsg("function \"%s\" already exists with same
argument types",
                            procedureName)));
        /*
         * It is possible that by the time we acquire the lock on function,
         * concurrent DDL has removed it. We can test this by checking the
         * existence of function. We get the tuple again to avoid the risk
         * of function definition getting changed.
         */
        if (!ConditionalLockDatabaseObject(ProcedureRelationId,
oldproc->oid, 0, AccessExclusiveLock))
            oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
                                        PointerGetDatum(procedureName),
                                        PointerGetDatum(parameterTypes),
                                        ObjectIdGetDatum(procNamespace));
    }