stac-utils / pgstac

Schema, functions and a python library for storing and accessing STAC collections and items in PostgreSQL
MIT License
145 stars 34 forks source link

Migration fails if postgis is already installed on a non-public schema #170

Open lhadjchikh opened 1 year ago

lhadjchikh commented 1 year ago

First of all, thanks for your work on this project! I'm looking forward to using it.

I'm trying to run migrations on an existing PostGIS database in which the postgis extension is installed on a schema called postgis. The database's current SEARCH_PATH includes this schema, but it's being overwritten by the following code:

https://github.com/stac-utils/pgstac/blob/main/src/pgstac/sql/000_idempotent_pre.sql#L101-L103 https://github.com/stac-utils/pgstac/blob/main/src/pgstac/sql/000_idempotent_pre.sql#L117 https://github.com/stac-utils/pgstac/blob/main/src/pgstac/sql/005_tileutils.sql#L1 https://github.com/stac-utils/pgstac/blob/main/src/pgstac/sql/006_tilesearch.sql#L1

The overwritten SEARCH_PATH causes the migration to fail because it can no longer find the geometry type in the postgis schema. It also removes other existing schemas from my current SEARCH_PATH, which breaks existing queries that use those schemas.

Rather than completely overwriting the existing SEARCH_PATH, would it be possible to just append the pgstac schema to the current SEARCH_PATH if it isn't included already? E.g.:

DO $$
  DECLARE
    search_path varchar := (SELECT current_setting('search_path'));
  BEGIN
    IF search_path NOT LIKE '%pgstac%' THEN
      SELECT set_config('search_path', 'pgstac,'||current_setting('search_path'), false);
    END IF;
  END
$$;

If the postgis extension is already installed on a non-public schema, it would also be necessary to ensure that pgstac has permission to use that schema, e.g.:

DO $$
  DECLARE
    postgis_schema varchar := (
      SELECT nspname FROM pg_namespace
      JOIN pg_extension ON pg_namespace.oid = extnamespace
      WHERE extname='postgis'
    );
  BEGIN
    EXECUTE format('GRANT USAGE ON SCHEMA %s TO pgstac_admin', postgis_schema);
  END
$$;

I'm not a SQL expert, so there may be a more elegant way to do this using a CTE.

In any case, it would be nice to provide some way to migrate an existing PostGIS database regardless of where the postgis extension was installed, and to not remove any schemas from the current SEARCH_PATH. Failing that, it would be helpful to update the documentation to indicate that the postgis extension needs to be installed on the public schema for the migrations to work, and to warn users that migrating will change the current SEARCH_PATH of their database.

bitner commented 1 year ago

@lhadjchikh Thanks for bringing this up!

The inline function that you created could be turned into a callable PLPGSQL function that could just be run at the top of each function:

CREATE OR REPLACE FUNCTION pgstac_path() RETURNS text AS $$
  DECLARE
    search_path varchar := (SELECT current_setting('search_path'));
  BEGIN
    IF search_path NOT LIKE '%pgstac%' THEN
      SELECT set_config('search_path', 'pgstac,'||current_setting('search_path'), false);
    END IF;
    RETURN current_setting('search_path');
  END;
$$ LANGUAGE PLPGSQL;

The other option is to schema qualify all calls to pgstac items (tables, views, other functions) within the pgstac function definitions (ie rather than things like SELECT FROM items, we can call SELECT FROM pgstac.items).

If a non-standard (ie non-public which also implies that the public role doesn't have access) PostGIS installation is present, we should absolutely make sure that we can add that to the search_path, but I don't think we should do anything to change permissions on items that are outside the pgstac schema. If someone is using a non-default PostGIS environment, that is likely because they want some further control and we should not override that automatically.

It is not a priority of mine right now to clean up the search_paths, but I would be very happy to help with and review any PRs that do so.