Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, 27 May 2025 10:03:58 +0800
jian he <jian(dot)universality(at)gmail(dot)com> wrote:

> On Tue, May 27, 2025 at 1:35 AM Yugo Nagata <nagata(at)sraoss(dot)co(dot)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));
> }

No. This cannot prevent the error "ERROR: tuple concurrently updated"
because it doesn't wait for end of the concurrently running session
if the lock cannot be acquired.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>