Re: v12 and pg_restore -f-

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: v12 and pg_restore -f-

Tom Lane-2
[ redirecting to -hackers ]

Justin Pryzby <[hidden email]> writes:
> I saw this and updated our scripts with pg_restore -f-

> https://www.postgresql.org/docs/12/release-12.html
> |In pg_restore, require specification of -f - to send the dump contents to standard output (Euler Taveira)
> |Previously, this happened by default if no destination was specified, but that was deemed to be unfriendly.

> What I didn't realize at first is that -f- has no special meaning in v11 - it
> just writes a file called ./-

Ugh.  I didn't realize that either, or I would have made a stink about
this patch.  Reducing the risk of getting a dump spewed at you is
completely not worth the cost of making it impossible to have
cross-version-compatible scripting of pg_restore.

Perhaps we could change the back branches so that they interpret "-f -"
as "write to stdout", but without enforcing that you use that syntax.
Nobody is going to wish that to mean "write to a file named '-'", so
I don't think this would be an unacceptable change.

Alternatively, we could revert the v12 behavior change.  On the whole
that might be the wiser course.  I do not think the costs and benefits
of this change were all that carefully thought through.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: v12 and pg_restore -f-

Andrew Gierth
>>>>> "Tom" == Tom Lane <[hidden email]> writes:

 Tom> Perhaps we could change the back branches so that they interpret
 Tom> "-f -" as "write to stdout", but without enforcing that you use
 Tom> that syntax.

We should definitely do that.

 Tom> Alternatively, we could revert the v12 behavior change. On the
 Tom> whole that might be the wiser course. I do not think the costs and
 Tom> benefits of this change were all that carefully thought through.

Failing to specify -d is a _really fricking common_ mistake for
inexperienced users, who may not realize that the fact that they're
seeing a ton of SQL on their terminal is not the normal result.
Seriously, this comes up on a regular basis on IRC (which is why I
suggested initially that we should do something about it).

--
Andrew (irc:RhodiumToad)


Reply | Threaded
Open this post in threaded view
|

Re: v12 and pg_restore -f-

Tom Lane-2
Andrew Gierth <[hidden email]> writes:
> "Tom" == Tom Lane <[hidden email]> writes:
>  Tom> Perhaps we could change the back branches so that they interpret
>  Tom> "-f -" as "write to stdout", but without enforcing that you use
>  Tom> that syntax.

> We should definitely do that.

>  Tom> Alternatively, we could revert the v12 behavior change. On the
>  Tom> whole that might be the wiser course. I do not think the costs and
>  Tom> benefits of this change were all that carefully thought through.

> Failing to specify -d is a _really fricking common_ mistake for
> inexperienced users, who may not realize that the fact that they're
> seeing a ton of SQL on their terminal is not the normal result.
> Seriously, this comes up on a regular basis on IRC (which is why I
> suggested initially that we should do something about it).

No doubt, but that seems like a really poor excuse for breaking
maintenance scripts in a way that basically can't be fixed.  Even
with the change suggested above, scripts couldn't rely on "-f -"
working anytime soon, because you couldn't be sure whether a
back-rev pg_restore had the update or not.

The idea I'm leaning to after more thought is that we should change
*all* the branches to accept "-f -", but not throw an error if you
don't use it.  Several years from now, we could put the error back in;
but not until there's a plausible argument that nobody is running
old versions of pg_restore anymore.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: v12 and pg_restore -f-

Andrew Gierth
BTW, the prior discussion is here:

https://www.postgresql.org/message-id/24868.1550106683%40sss.pgh.pa.us

--
Andrew (irc:RhodiumToad)


Reply | Threaded
Open this post in threaded view
|

Re: v12 and pg_restore -f-

Stephen Frost
In reply to this post by Tom Lane-2
Greetings,

* Tom Lane ([hidden email]) wrote:
> Andrew Gierth <[hidden email]> writes:
> > "Tom" == Tom Lane <[hidden email]> writes:
> >  Tom> Perhaps we could change the back branches so that they interpret
> >  Tom> "-f -" as "write to stdout", but without enforcing that you use
> >  Tom> that syntax.
>
> > We should definitely do that.

I agree that this would be a reasonable course of action.  Really, it
should have always meant that...

> >  Tom> Alternatively, we could revert the v12 behavior change. On the
> >  Tom> whole that might be the wiser course. I do not think the costs and
> >  Tom> benefits of this change were all that carefully thought through.
>
> > Failing to specify -d is a _really fricking common_ mistake for
> > inexperienced users, who may not realize that the fact that they're
> > seeing a ton of SQL on their terminal is not the normal result.
> > Seriously, this comes up on a regular basis on IRC (which is why I
> > suggested initially that we should do something about it).
>
> No doubt, but that seems like a really poor excuse for breaking
> maintenance scripts in a way that basically can't be fixed.  Even
> with the change suggested above, scripts couldn't rely on "-f -"
> working anytime soon, because you couldn't be sure whether a
> back-rev pg_restore had the update or not.
Maintenance scripts break across major versions.  We completely
demolished everything around how recovery works, and some idea that you
could craft up something easy that would work in a backwards-compatible
way is outright ridiculous, so I don't see why we're so concerned about
a change to how pg_restore works here.

> The idea I'm leaning to after more thought is that we should change
> *all* the branches to accept "-f -", but not throw an error if you
> don't use it.  Several years from now, we could put the error back in;
> but not until there's a plausible argument that nobody is running
> old versions of pg_restore anymore.

No, I don't agree with this, at all.

Thanks,

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: v12 and pg_restore -f-

Euler Taveira
Em ter, 8 de out de 2019 às 15:08, Stephen Frost <[hidden email]> escreveu:

>
> Greetings,
>
> * Tom Lane ([hidden email]) wrote:
> > Andrew Gierth <[hidden email]> writes:
> > > "Tom" == Tom Lane <[hidden email]> writes:
> > >  Tom> Perhaps we could change the back branches so that they interpret
> > >  Tom> "-f -" as "write to stdout", but without enforcing that you use
> > >  Tom> that syntax.
> >
> > > We should definitely do that.
>
> I agree that this would be a reasonable course of action.  Really, it
> should have always meant that...
>
Indeed, it was a broken behavior and the idea was to fix it. However,
changing pg_restore in back-branches is worse than do nothing because
it could break existent scripts.

> > >  Tom> Alternatively, we could revert the v12 behavior change. On the
> > >  Tom> whole that might be the wiser course. I do not think the costs and
> > >  Tom> benefits of this change were all that carefully thought through.
> >
> > > Failing to specify -d is a _really fricking common_ mistake for
> > > inexperienced users, who may not realize that the fact that they're
> > > seeing a ton of SQL on their terminal is not the normal result.
> > > Seriously, this comes up on a regular basis on IRC (which is why I
> > > suggested initially that we should do something about it).
> >
> > No doubt, but that seems like a really poor excuse for breaking
> > maintenance scripts in a way that basically can't be fixed.  Even
> > with the change suggested above, scripts couldn't rely on "-f -"
> > working anytime soon, because you couldn't be sure whether a
> > back-rev pg_restore had the update or not.
>
> Maintenance scripts break across major versions.  We completely
> demolished everything around how recovery works, and some idea that you
> could craft up something easy that would work in a backwards-compatible
> way is outright ridiculous, so I don't see why we're so concerned about
> a change to how pg_restore works here.
>
Yeah, if you check pg_restore version, you could use new syntax for
12+. We break scripts every release (mainly with catalog changes) and
I don't know why this change is different than the other ones. The
pg_restore changes is more user-friendly and less error-prone.


Regards,

--
   Euler Taveira                                   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


Reply | Threaded
Open this post in threaded view
|

Re: v12 and pg_restore -f-

Stephen Frost
Greetings,

* Euler Taveira ([hidden email]) wrote:

> Em ter, 8 de out de 2019 às 15:08, Stephen Frost <[hidden email]> escreveu:
> > * Tom Lane ([hidden email]) wrote:
> > > Andrew Gierth <[hidden email]> writes:
> > > > "Tom" == Tom Lane <[hidden email]> writes:
> > > >  Tom> Perhaps we could change the back branches so that they interpret
> > > >  Tom> "-f -" as "write to stdout", but without enforcing that you use
> > > >  Tom> that syntax.
> > >
> > > > We should definitely do that.
> >
> > I agree that this would be a reasonable course of action.  Really, it
> > should have always meant that...
> >
> Indeed, it was a broken behavior and the idea was to fix it. However,
> changing pg_restore in back-branches is worse than do nothing because
> it could break existent scripts.
I can certainly respect that argument, in general, but in this specific
case, I've got a really hard time believeing that people wrote scripts
which use '-f -' with the expectation that a './-' file was to be
created.

Thanks,

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: v12 and pg_restore -f-

imai.yoshikazu@fujitsu.com
Hi,

On Sun, Oct 6, 2019 at 7:09 PM, Justin Pryzby wrote:
> I saw this and updated our scripts with pg_restore -f-
> https://www.postgresql.org/docs/12/release-12.html
> |In pg_restore, require specification of -f - to send the dump contents to standard output (Euler Taveira)
> |Previously, this happened by default if no destination was specified, but that was deemed to be unfriendly.
>
> What I didn't realize at first is that -f- has no special meaning in v11 - it
> just writes a file called ./-  And it's considered untennable to change
behavior of v11.

Ahh... I totally missed thinking about the behavior of "-f -" in v11 when I reviewed this patch.


On Wed, Oct 9, 2019 at 0:45 PM, Stephen Frost wrote:

> * Euler Taveira ([hidden email]) wrote:
> > Em ter, 8 de out de 2019 às 15:08, Stephen Frost <[hidden email]> escreveu:
> > > * Tom Lane ([hidden email]) wrote:
> > > > Andrew Gierth <[hidden email]> writes:
> > > > > "Tom" == Tom Lane <[hidden email]> writes:
> > > > >  Tom> Perhaps we could change the back branches so that they
> > > > > interpret  Tom> "-f -" as "write to stdout", but without
> > > > > enforcing that you use  Tom> that syntax.
> > > >
> > > > > We should definitely do that.
> > >
> > > I agree that this would be a reasonable course of action.  Really,
> > > it should have always meant that...
> > >
> > Indeed, it was a broken behavior and the idea was to fix it. However,
> > changing pg_restore in back-branches is worse than do nothing because
> > it could break existent scripts.
>
> I can certainly respect that argument, in general, but in this specific case, I've got a really hard time believeing
> that people wrote scripts which use '-f -' with the expectation that a './-' file was to be created.

+1.

If we only think of the problem that we can't use "-f -" with the meaning "dump to the stdout" in v11 and before ones, it seems a bug and we should fix it.
Of course, if we fix it, some people would go into the trouble, but such people are who wrote scripts which use '-f -' with the expectation that a './-' file.
I don't think there are such people a lot.


--
Yoshikazu Imai