Re: erroneous restore into pg_catalog schema - Mailing list pgsql-hackers

From Tom Lane
Subject Re: erroneous restore into pg_catalog schema
Date
Msg-id 12365.1358098148@sss.pgh.pa.us
Whole thread Raw
In response to erroneous restore into pg_catalog schema  ("Erik Rijkers" <er@xs4all.nl>)
Responses Re: erroneous restore into pg_catalog schema
Re: erroneous restore into pg_catalog schema
List pgsql-hackers
"Erik Rijkers" <er@xs4all.nl> writes:
> If you dump a table with -t schema.table, and in the receiving database that schema does not
> exist, pg_restore-9.3devel will restore into the pg_catalog schema:
> ...
> Off course the workaround is obvious, but shouldn't this be prevented from happening in the first
> place?  9.2 refuses to do such a restore, which seems much better.

I said to myself "huh?  surely we did not change pg_dump's behavior
here".  But actually it's true, and the culprit is commit 880bfc328,
"Silently ignore any nonexistent schemas that are listed in
search_path".  What pg_dump is emitting is
SET search_path = s, pg_catalog;CREATE TABLE t (...);

and in HEAD the SET throws no error and instead establishes pg_catalog
as the target schema for object creation.  Oops.

So obviously, 880bfc328 was several bricks shy of a load.  The arguments
for that change in behavior still seem good for schemas *after* the
first one; but the situation is entirely different for the first schema,
because that's what the command is attempting to establish as the
creation schema.

In hindsight it might've been better if we'd used a separate GUC for the
object creation schema, but it's much too late to change that.

When we're dealing with a client-supplied SET command, I think it'd be
okay to just throw an error if the first schema doesn't exist.  However,
the main issue in the discussion leading up to 880bfc328 was how to
behave for search_path values coming from noninteractive sources such as
postgresql.conf.  In such cases we really don't have much choice about
whether to adopt the value in some sense.

I think maybe what we should do is have namespace.c retain an explicit
notion that "the first schema listed in search_path didn't exist", and
then throw errors if any attempt is made to create objects without an
explicitly specified namespace.

If we did that then we'd have a choice about whether to throw error in
the interactive-SET case.  I'm a bit inclined to let it pass with no
error for consistency with the non-interactive case; IIRC such
consistency was one of the arguments made in the previous discussion.
But certainly there's an argument to be made the other way, too.

Thoughts?
        regards, tom lane



pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: [Pgbuildfarm-members] Version 4.10 of buildfarm client released.
Next
From: Pavel Stehule
Date:
Subject: Re: ToDo: log plans of cancelled queries