Patch to avoid orphaned dependencies - Mailing list pgsql-hackers

Hi,

This new thread is a follow-up of [1].

Problem description:

We have occasionally observed objects having an orphaned dependency, the most common case we have seen (if not the only one) is functions not linked to any namespaces.
A patch has been initially proposed to fix this particular (function-to-namespace) dependency (see [1]), but there could be much more scenarios (like the function-to-datatype one highlighted by Gilles in [1] that could lead to a function having an invalid parameter datatype).
As Tom said there are dozens more cases that would need to be considered, and a global approach to avoid those race conditions should be considered instead.

The attached patch is avoiding those race conditions globally by changing the dependency mechanism: we are using a dirty snapshot any time we’re about to create a pg_depend or pg_shdepend entry.
That way we can check if there is in-flight transactions that are affecting the dependency: if that’s the case, an error is being reported.

This approach has been chosen over another one that would have make use of the locking machinery.
The reason for this choice is to avoid possible slow down of typical DDL command, risk of deadlock, number of locks taken by transaction...

Implementation overview:

• A new catalog snapshot is added: DirtyCatalogSnapshot.
• This catalog snapshot is a dirty one to be able to look for in-flight dependencies.
• Its usage is controlled by a new UseDirtyCatalogSnapshot variable.
• Any time this variable is being set to true, then the next call to GetNonHistoricCatalogSnapshot() is returning the dirty snapshot.
• This snapshot is being used to check for in-flight dependencies and also to get the objects description to generate the error messages.

Testing:

Test 1

Session1:

postgres=# create schema tobeorph;
CREATE SCHEMA
postgres=# create table tobeorph.bdt (a int);
CREATE TABLE
postgres=# begin;
BEGIN
postgres=*# CREATE OR REPLACE FUNCTION tobeorph.bdttime() RETURNS TIMESTAMP AS $$
DECLARE   outTS TIMESTAMP;
BEGIN   perform pg_sleep(10);   RETURN CURRENT_TIMESTAMP;
END;
$$ LANGUAGE plpgsql volatile;
CREATE FUNCTION

Session 1 does not commit, then session 2:

postgres=# drop schema tobeorph;
ERROR:  cannot drop schema tobeorph because other objects depend on it
DETAIL:  table tobeorph.bdt depends on schema tobeorph
function tobeorph.bdttime() (not yet committed) depends on schema tobeorph
HINT:  DROP and DROP CASCADE won't work when there are uncommitted dependencies.

Test 2

Session 1:

postgres=# create schema toinsert;
CREATE SCHEMA
postgres=# begin;
BEGIN
postgres=*# drop schema toinsert;
DROP SCHEMA

Session 1 does not commit, then session 2:

postgres=# CREATE OR REPLACE FUNCTION toinsert.bdttime() RETURNS TIMESTAMP AS $$
DECLARE   outTS TIMESTAMP;
BEGIN   perform pg_sleep(10);   RETURN CURRENT_TIMESTAMP;
END;
$$ LANGUAGE plpgsql volatile;
ERROR:  cannot create function toinsert.bdttime() because it depends of other objects uncommitted dependencies
DETAIL:  function toinsert.bdttime() depends on schema toinsert (dependency not yet committed)
HINT:  CREATE won't work as long as there is uncommitted dependencies.

Test3

Session1:
psql -U toorph postgres
psql (14devel)
Type "help" for help.

postgres=> begin;
BEGIN
postgres=*> CREATE OR REPLACE FUNCTION bdttime() RETURNS TIMESTAMP AS $$
DECLARE   outTS TIMESTAMP;
BEGIN   perform pg_sleep(10);   RETURN CURRENT_TIMESTAMP;
END;
$$ LANGUAGE plpgsql volatile;
CREATE FUNCTION

Session 1 does not commit, then session 2:

postgres=# drop owned by toorph;
ERROR:  cannot drop objects owned by role toorph because other uncommitted objects depend on it
DETAIL:  function public.bdttime() (not yet committed) depends on role toorph
HINT:  Commit or rollback function public.bdttime() creation.

I'm creating a new commitfest entry for this patch.

Thanks

Bertrand

[1]: https://www.postgresql.org/message-id/flat/a4f55089-7cbd-fe5d-a9bb-19adc6418ae9%40darold.net#9af5cdaa9e80879beb1def3604c976e8