Re: BUG #14650: pg_dump -c fails when 'public' schema doesn't exist

On Mon, May 15, 2017 at 12:40 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Michael Paquier (michael(dot)paquier(at)gmail(dot)com) wrote:
>> On Sat, May 13, 2017 at 3:22 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > tureba(at)gmail(dot)com writes:
>> >> It seems that pg_dump -c fails when the database doesn't have a 'public'
>> >> schema. When using a clean database, then dropping (or renaming) the
>> >> 'public' schema, a subsequent pg_dump -c tells me:
>> >
>> >> $ pg_dump -c test
>> >> pg_dump: [archiver (db)] query failed: ERROR: schema "public" does not exist
>> >> LINE 1: ...::regclass AND pip.objsubid = 0 AND pip.objoid <> 'public'::...
>> >> ^
>> >
>> >> This seems to have started in commit 330b84d8c4
>> >
>> > Yeah, this is not cool. Stephen?
>>
>> This is visibly an oversight of the recent commit 330b84d, which is
>> broken on its face because it does not check for the existence of this
>> schema. One simple fix would be to extend this query with a SELECT
>> CASE ... ELSE 0 FROM pg_namespace WHERE nspname = 'public' but that's
>> ugly.
>
> I would think using a coalesce() would be a bit cleaner, as in:
>
> ! appendPQExpBuffer(query,
> ! " AND pip.objoid <> "
> ! "coalesce((select oid from pg_namespace "
> ! "where nspname = 'public'),0)");

No objections to that.

> Attached is a patch which fixes this issue in that way and includes
> changes to the TAP tests to check this case.

This looks good to me.

> The TAP tests are a bit
> much to back-patch, I guess, since it involves adding the ability to
> have multiple databases to the 002_pg_dump.pl framework, but hopefully
> there won't be objections to adding that into master. This may also
> provide an initial step towards folding 010_dump_connstr.pl into
> 002_pg_dump.pl and therefore possibly removing the need for another
> cluster to be created during the pg_dump TAP tests and hopefully
> reducing their overall runtime.

Back-patching test-related patches are not a bad thing IMO, this
provides more coverage for bugs in stable releases.
--
Michael