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 21:52:24
Message-ID: CAKOSWNnpuPPGpxeE3tBNEDrZ9fe6iVmgJc0u+eQX90-k6304dg@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:
> Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com> writes:
>> On 9/27/16, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I'm not exactly convinced that you did. There's only one copy of
>>> Archive->remoteVersion, and you're overwriting it long before the
>>> dump process is over.
>
>> It does not seem that I'm "overwriting it long before the dump process
>> is over"...
>
> There's a lot that happens during RestoreArchive. Even if none of it
> inspects remoteVersion today, I do not think that's a safe assumption to
> make going forward.

And... Khm... Note that even _now_ AHX->remoteVersion is set to a
database version pg_restore connects to... So all the code has it
during restoring process...

> The easiest counterexample is that this very bit of
> code you want to add does so.

The only change I've done is set remoteVersion to the maximum allowed
when output is the plain text format.

> I really do not want to get into a design
> that says "remoteVersion means the source server version until we reach
> RestoreArchive, and the target version afterwards". That way madness lies.

It is only if you think about "remoteVersion" as
"sourceServerVersion", but even now it is not so.

Moreover RestoreArchive is a delimter only for pg_dump and only when
output is a plain text.
For other modes of the pg_dump RestoreArchive is not called at all.

> If we're going to try altering the emitted SQL based on target version,
> let's first create a separation between those concepts;

I've just found there is _archiveHandle.archiveRemoteVersion. Is it a
parameter you were searched for?
The pg_restore code does not use it (just as remoteVersion), but it
can do so if it is necessary.

> otherwise I will bet that we add more bugs than we remove.
>
> (The other thing I'd want here is a --target-version option so that
> you could get the same output alterations in pg_dump or pg_restore to
> text. Otherwise it's nigh undebuggable, and certainly much harder
> to test than it needs to be.)

I thought that way. I'm ready to introduce that parameter, but again,
I see now it will influence only SET parameters. Does it worth it?

--
Best regards,
Vitaly Burovoy

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-09-27 22:02:27 Re: LLVM Address Sanitizer (ASAN) and valgrind support
Previous Message Stephen Frost 2016-09-27 21:17:05 Re: Add support for restrictive RLS policies