aws / pg_tle

Framework for building trusted language extensions for PostgreSQL
Apache License 2.0
337 stars 31 forks source link

pg_dump/pg_restore with custom datatype fails unless `--use-set-session-authorization` is specified #249

Closed adamguo0 closed 1 year ago

adamguo0 commented 1 year ago

Description

pg_tle custom datatypes fail to pg_dump and pg_restore. The pg_restore fails with the error:

ERROR:  ALTER or REPLACE of pg_tle used datatype I/O function test_citext_in is not allowed

If --use-set-session-authorization is specified when running pg_dump, then the dump file does not try to ALTER the function's OWNER and the restore succeeds, but this requires superuser privileges.

Steps to reproduce

  1. Create a custom data type
    
    CREATE EXTENSION pg_tle;
    CREATE DATABASE restore;
    SELECT pgtle.create_shell_type('public', 'test_citext');

CREATE FUNCTION public.test_citext_in(input text) RETURNS bytea AS $$ SELECT pg_catalog.convert_to(input, 'UTF8'); $$ IMMUTABLE STRICT LANGUAGE sql; CREATE FUNCTION public.test_citext_out(input bytea) RETURNS text AS $$ SELECT pg_catalog.convert_from(input, 'UTF8'); $$ IMMUTABLE STRICT LANGUAGE sql;

SELECT pgtle.create_base_type( 'public', 'test_citext', 'test_citext_in(text)'::regprocedure, 'test_citext_out(bytea)'::regprocedure, -1 );

2. Run `pg_dump -f custom_datatype_error.sql -d postgres`
3. Run `psql -d restore -f test.sql`

## Expected outcome

`psql -d restore -f test.sql` succeeds without any errors

## Actual outcome

`psql -d restore -f test.sql` outputs the following errors:

ERROR: ALTER or REPLACE of pg_tle used datatype I/O function test_citext_in is not allowed ERROR: ALTER or REPLACE of pg_tle used datatype I/O function test_citext_out is not allowed



## Analysis

We made the decision (https://github.com/aws/pg_tle/pull/219) to block ALTER FUNCTION OWNER of custom datatype functions except during binary upgrades -- we should consider adding more exceptions or just removing it altogether https://github.com/aws/pg_tle/blob/main/src/tleextension.c#L4314-L4330

Also, we have existing tests to catch pg_dump/pg_restore issues with custom datatypes, but the tests are incorrectly passing despite this error.

1. The test runs `psql` to restore the dump file and expects a exit code of 0. However `psql` exits with code 0 even if individual queries failed. Hence the test wrongly thinks that the restore is successful. https://github.com/aws/pg_tle/blob/main/test/t/002_pg_tle_dump_restore.pl#L299-L303
2. In this test it happens that ALTER OWNER is not strictly needed, because the role that creates the custom datatype function and the role that restores the dump are the same. When the test tries to exercise the custom datatype, it succeeded regardless of the earlier restore error.

We can fix 1. by adding `-v ON_ERROR_STOP=1` to the `psql` restore command, which causes the test to fail at the attempted restore.
adamguo0 commented 1 year ago

We can add an exception for superuser to alter function owner. pg_restore won't succeed for non-superuser roles regardless since create extension pg_tle requires superuser