Re: Detect supported SET parameters when pg_restore is run

From: Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Detect supported SET parameters when pg_restore is run
Date: 2016-09-27 17:32:57
Message-ID: CAKOSWNnBUg-=5LR95HWVnh+sLUqxf4V3KCnvqOBgm+Xbf55EFA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 9/27/16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Mon, Sep 26, 2016 at 9:56 PM, Vitaly Burovoy
>> <vitaly(dot)burovoy(at)gmail(dot)com> wrote:
>>> We do dump/restore schemas/data via custom/dir formats and we have to
>>> keep several client versions for 9.2, 9.4 and 9.5 versions on local
>>> workstations because after pg_restore95 connects to 9.2, it fails when
>>> it sets run-time parameters via SET:
>
>> I think our policy is that a newer pg_dump needs to work with an older
>> version of the database, but I don't think we make any similar
>> guarantee for other tools, such as pg_restore. It's an interesting
>> question whether we should try a little harder to do that, but I
>> suspect it might require more changes than what you've got in this
>> patch....

Well... I'm not inclined to insert support of restoring from a higher
major version to a lower one, because it can lead to security issues
(e.g. with RLS). But my observation is that all new features supported
by the "pg_dump" are "incremental", e.g. the last feature "parallel"
for pg_dump/pg_restore --- lack of "PARALLEL UNSAFE" (which is by
default) from 9.6 and lack of it from pre-9.6.

That behavior allows newer versions of pg_restore to use dumps from DB
of older versions because of lack of new features grammar. With the
patch I'm able to use pg_dump96/pg_restore96 for our database of 9.2
(dump from 9.2 and restore to 9.2).

> The general policy has always been that pg_dump output is only expected to
> restore without errors into a server that's the same or newer version as
> pg_dump (regardless of the server version being dumped from). If you try
> to restore into an older server version, you may get some errors, which we
> try to minimize the scope of when possible. But it will never be possible
> to promise none at all. I think the correct advice here is simply "don't
> use pg_restore -e -1 when trying to restore into an older server version".

Why can't I use it if pg_dump92/pg_restore92 have the same behavior as
pg_dump96/pg_restore96 except the SET block?
The patch does not give guarantee of a restoration, it just avoids
setting unsupported parameters for pg_restore the same way as pg_dump
does.
The other issues are for solving by the user who wants to restore to a
DB of older version.

> Taking a step back, there are any number of ways that you might get
> errors during a pg_restore into a DB that's not set up exactly as pg_dump
> expected. Missing roles or tablespaces, for example.

It does not depends on a pg_dump/pg_restore version and can be soved
by using command line options:
--no-tablespaces --no-owner --no-privileges

> Again, the dump output is constructed so that you can survive those problems
> and bull ahead ... but not with "-e -1".

I think "-e -1" was invented specially for it --- stop restoring if
something is going wrong. Wasn't it?

> I don't see a very good reason why
> older-server-version shouldn't be considered the same type of problem.
>
> The patch as given seems rather broken anyway --- won't it change text
> output from pg_dump as well as on-line output from pg_restore?

My opinion --- no (and I wrote it in the initial letter): because it
is impossible to know what version of a database is used for that
plain text output. Users who use output to a plain text are able to
use sed (or something else) to delete unwanted rows.

> (That is,
> it looks to me like the SETs emitted by pg_dump to text format would
> depend on the source server version, which they absolutely should not.
> Either that, or the patch is overwriting pg_dump's idea of what the
> source server version is at the start of the output phase, which is
> likely to break all kinds of stuff when dumping from an older server.)

I agree, that's why I left current behavior as is for the plain text output.

> It's possible that we could alleviate this specific symptom by arranging
> for the BEGIN caused by "-1" to come out after the initial SETs, but
> I'm not sure how complicated that would be or whether it's worth the
> trouble.

It leads to change ERRORs to WARNINGs but current behavior discussed
above is left the same.
Why don't just avoid SET parameters when we know for sure they are not
supported by the server to which pg_restore is connected?

--
Best regards,
Vitaly Burovoy

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2016-09-27 17:35:49 Re: Floating point comparison inconsistencies of the geometric types
Previous Message Kevin Grittner 2016-09-27 17:29:45 Re: Misdesigned command/status APIs for parallel dump/restore