Re: [UNVERIFIED SENDER] Re: [BUG] orphaned function

I ended up making more changes in QualifiedNameGetCreationNamespace() to mimic the retry() logic that is in RangeVarGetAndCheckCreationNamespace().

The attached v2 patch, now protects the function to be orphaned in those 2 scenarios:

Scenario 1: first, session 1 begin create function, then session 2 drops the schema: drop schema is locked and would produce (once the create function finishes):

bdt=# 2020-12-02 10:12:46.364 UTC [15810] ERROR:  cannot drop schema tobeorph because other objects depend on it
2020-12-02 10:12:46.364 UTC [15810] DETAIL:  function tobeorph.bdttime() depends on schema tobeorph
2020-12-02 10:12:46.364 UTC [15810] HINT:  Use DROP ... CASCADE to drop the dependent objects too.
2020-12-02 10:12:46.364 UTC [15810] STATEMENT:  drop schema tobeorph;

Scenario 2: first, session 1 begin drop schema, then session 2 creates the function: create function is locked and would produce (once the drop schema finishes):

2020-12-02 10:14:29.468 UTC [15822] ERROR:  schema "tobeorph" does not exist

Thanks

Bertrand

Hi,

This is a review for the orphaned function bug fix https://www.postgresql.org/message-id/flat/5a9daaae-5538-b209-6279-e903c3ea2157@amazon.com

Contents & Purpose
==================

This patch fixes an historical race condition in PostgreSQL that leave a function orphan of a namespace. It happens when a function is created in a namespace inside a transaction not yet committed and that another session drop the namespace. The function become orphan and can not be used anymore.

postgres=# \df *.bdttime
                List of functions
  Schema |  Name   |      Result data type       | Argument data types | Type
 --------+---------+-----------------------------+---------------------+------
         | bdttime | timestamp without time zone |                     | func

Initial Run
===========

The patch applies cleanly to HEAD. The regression tests all pass successfully with the new behavior. Given the example of the case to reproduce, the second session now waits until the first session is committed and when it is done it thrown error:

    postgres=# drop schema tobeorph;
    ERROR:  cannot drop schema tobeorph because other objects depend on it
    DETAIL:  function tobeorph.bdttime() depends on schema tobeorph
    HINT:  Use DROP ... CASCADE to drop the dependent objects too.

the function is well defined in the catalog.

Comments
========

As Tom mention there is still pending DDL concurrency problems. For example if session 1 execute the following:

    CREATE TYPE public.foo as enum ('one', 'two');
    BEGIN;
    CREATE OR REPLACE FUNCTION tobeorph.bdttime(num foo) RETURNS TIMESTAMP AS $$
    DECLARE
       outTS TIMESTAMP;
    BEGIN
       perform pg_sleep(10);
       RETURN CURRENT_TIMESTAMP;
    END;
    $$ LANGUAGE plpgsql volatile;

if session 2 drop the data type before the session 1 is committed, the function will be declared with invalid parameter data type.

The same problem applies if the returned type or the procedural language is dropped. I have tried to fix that in ProcedureCreate() by using an AccessSharedLock for the data types and languages involved in the function declaration. With this patch the race condition with parameters types, return types and PL languages are fixed. Only data types and procedural languages with Oid greater than FirstBootstrapObjectId will be locked locked. But this is probably more complex than this fix so it is proposed as a separate patch (v1-003-orphaned_function_types_language.patch) to not interfere with the applying of Bertran's patch.


Conclusion
==========

I tag this patch (v1-002-orphaned_function.patch) as ready for committers review as it fixes the bug reported.

-- 
Gilles Darold
LzLabs GmbH
https://www.lzlabs.com/