fix: isolate pg_net to pg_catalog extension

[AndrewJackson2020]

Currently pg_net hardcodes everything to a net schema. This schema is created in the sql scripts and is owned by the extension. Even though it has its own dedicated schema you still need to install pg_net in another schema. This ends up with no objects in that schema but it "exists" in that schema for recordkeeping purposes. This commit changes this so that pg_net is declared with the net extension in the control file.

All this being said, I think the ideal behavior is to make pg_net a relocatable extension. There are some difficulties here in that the C source code assumes the net schemas installation. That said I feel like it is still possible. Happy to accept any feedback here.

[AndrewJackson2020]

Looking into this a bit more, postgis is an example of an extension that uses tables, like pg_net. Unlike pg_net, instead of hardcoding the schema name it dynamically looks up the location of the extension with the below function. Given that, maybe the route we should go here is turning this into a relocatable exension.

/*
 * get_extension_schema - given an extension OID, fetch its extnamespace
 *
 * Returns InvalidOid if no such extension.
 */
static Oid
postgis_get_extension_schema(Oid ext_oid)
{
    Oid         result;
    SysScanDesc scandesc;
    HeapTuple   tuple;
    ScanKeyData entry[1];

    Relation rel = table_open(ExtensionRelationId, AccessShareLock);
    ScanKeyInit(&entry[0],
    	Anum_pg_extension_oid,
        BTEqualStrategyNumber, F_OIDEQ,
        ObjectIdGetDatum(ext_oid));

    scandesc = systable_beginscan(rel, ExtensionOidIndexId, true,
                                  NULL, 1, entry);

    tuple = systable_getnext(scandesc);

    /* We assume that there can be at most one matching tuple */
    if (HeapTupleIsValid(tuple))
        result = ((Form_pg_extension) GETSTRUCT(tuple))->extnamespace;
    else
        result = InvalidOid;

    systable_endscan(scandesc);

    table_close(rel, AccessShareLock);

    return result;
}

[AndrewJackson2020]

Another idea: pg_cron has a similar pattern as pg_net: creates a cron schema where everything is installed, non-relocatable, etc. They hardcode the installation to pg_catalog though. Maybe we should consider this as an alternative to having pg_cron be non-relocatable but allowing it to be installed anywhere?

https://github.com/citusdata/pg_cron/blob/5cedfa472ccc83567aa23ec645925ed8489a7797/pg_cron.control

[steve-chavez]

They hardcode the installation to pg_catalog though

Could you elaborate on the pros/cons of that? 👀

[AndrewJackson2020]

They hardcode the installation to pg_catalog though

Could you elaborate on the pros/cons of that? 👀

The most obvious one that I can come up with is that supabases's platform will throw a warning in the advisor UI if you accidentally install an extension in the public schema (image pasted below).

Some more reasons: you can drop public but you cannot drop pg_catalog [snippet 1]. Also pg_catalog is a special schema where you can't even modify outside of privileged mechanism like extension creation[snippet 2]. Also it seems more ergonomic from a UX perspective as we provide customers with a choice as to "what schema do you want to install this extension" while it really does not impact them.

My main motivation would just be to be consistent with the rest of the postgres community. Looking at procedural languages, they are all non relocatable and installed in pg_catalog only[snippet 3]. Interestingly contrib has a nonrelocatable extension where the schema is not specified[snippet 4] , this may be an oversight though.

snippet 1

13:21:12 aj@localhost:5432/postgres  40857 =#
drop schema pg_catalog ;
ERROR:  cannot drop schema pg_catalog because it is required by the database system
13:21:19 aj@localhost:5432/postgres  40857 =#
drop schema public ;
DROP SCHEMA

snippet 2

13:21:33 aj@localhost:5432/postgres  40857 =#
create table pg_catalog.whatever (whatever int);
ERROR:  permission denied to create "pg_catalog.whatever"
DETAIL:  System catalog modifications are currently disallowed.

snippet 3

aj@Andrews-MacBook-Pro:~/Repos/mono/external/repos/public/postgres/master/src/pl/ > rg --after-context=1 relocatable
plpgsql/src/plpgsql.control
5:relocatable = false
6-schema = pg_catalog

plperl/plperl.control
5:relocatable = false
6-schema = pg_catalog

plperl/plperlu.control
5:relocatable = false
6-schema = pg_catalog

tcl/pltclu.control
5:relocatable = false
6-schema = pg_catalog

plpython/plpython3u.control
5:relocatable = false
6-schema = pg_catalog

tcl/pltcl.control
5:relocatable = false
6-schema = pg_catalog

snippet 4

aj@Andrews-MacBook-Pro:~/Repos/mono/external/repos/public/postgres/master/contrib/ > cat xml2/xml2.control
# xml2 extension
comment = 'XPath querying and XSLT'
default_version = '1.2'
module_pathname = '$libdir/pgxml'
# XXX do we still need this to be non-relocatable?
relocatable = false

[AndrewJackson2020]

I have reworked this PR to install the extension in pg_catalog. Looking at the evidence above I think that this is the most correct option as this will make it behave like most other non relocatable extensions.

[steve-chavez]

Not opposed to non-relocatable being in pg_catalog, but I guess then #290 should be closed as "not planned"?

Also, this would be a breaking change right? Not sure yet of the implications for existing deployments, that'd be the most concerning part.

[AndrewJackson2020]

Not opposed to non-relocatable being in pg_catalog, but I guess then #290 should be closed as "not planned"?

Also, this would be a breaking change right? Not sure yet of the implications for existing deployments, that'd be the most concerning part.

Agree this is not something I would want to merge in immediately or whatever. Converting back to draft so this doesn't get merged.

Per the postgres docs this setting should only impact new extension installs.

This parameter can only be set for non-relocatable extensions. It forces the extension to be loaded into exactly the named schema and not any other. The schema parameter is consulted only when initially creating an extension, not during extension updates. See Section 36.17.2 for more information.

The supabase UI ideally should be changed though looking at the the current behavior I don't think it would need to be. Notice how even if I try to install in public it doesn't fail, just ignores and installs in pg_catalog without notice. This may even be not a breaking change.

15:25:50 postgres@db.macgszcnaryxuexcoobv.supabase.red:5432/postgres  377875 =>
create extension pg_cron with schema public;
CREATE EXTENSION
15:26:06 postgres@db.macgszcnaryxuexcoobv.supabase.red:5432/postgres  377875 =>
\dx pg_cron
                                            List of installed extensions
        Name        | Version |   Schema   |                              Description
--------------------+---------+------------+------------------------------------------------------------------------
 pg_cron            | 1.6.4   | pg_catalog | Job scheduler for PostgreSQL
(1 rows)

but I guess then #290 should be closed as "not planned"?

I'd be fine with it being not planned or being on the far back burner.

[0] https://www.postgresql.org/docs/current/extend-extensions.html
[1] https://github.com/supabase/supabase/blob/7f0f3c32b557b774323a800d8550f7cf6ce3bddc/apps/studio/components/interfaces/Database/Extensions/EnableExtensionModal.tsx#L88