Re: pg_dump incorrect output in plaintext mode

$ pg_dump -v postgres -Fc -t t > /dev/null
...
pg_dump: dumping contents of table t
...

but in plaintext:
$ pg_dump -v postgres -Fp -t t > /dev/null
pg_dump: creating TABLE t
pg_dump: restoring data for table "t"
pg_dump: dumping contents of table t
pg_dump: setting owner and privileges for TABLE t
pg_dump: setting owner and privileges for TABLE DATA t

In particular, the "restoring data" is obviously completely wrong. But
AFAICT, the "creating table" and "setting owner" etc are also wrong...

This is because pg_dump calls RestoreArchive(fout).

Do we need to pass down a parameter all the way down the functions to
tell them if they're restoring or not?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Excerpts from Magnus Hagander's message of mar ago 28 08:08:24 -0400 2012:

> In particular, the "restoring data" is obviously completely wrong. But
> AFAICT, the "creating table" and "setting owner" etc are also wrong...
>
> This is because pg_dump calls RestoreArchive(fout).
>
> Do we need to pass down a parameter all the way down the functions to
> tell them if they're restoring or not?

Hmm, but the info is already there, is it not? Isn't AH->ropt->useDB
what you want?

As a translator, I'm not sure that I'm too happy with the idea of having
to translate double the number of strings in pg_dump ...

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> but in plaintext:
> $ pg_dump -v postgres -Fp -t t > /dev/null
> pg_dump: creating TABLE t
> pg_dump: restoring data for table "t"
> pg_dump: dumping contents of table t
> pg_dump: setting owner and privileges for TABLE t
> pg_dump: setting owner and privileges for TABLE DATA t

I don't see anything particularly incorrect about that. The point of
the --verbose switch is to track what pg_dump is doing, and if what
it's doing involves going through RestoreArchive(), why should we try
to hide the fact?

regards, tom lane

On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> but in plaintext:
>> $ pg_dump -v postgres -Fp -t t > /dev/null
>> pg_dump: creating TABLE t
>> pg_dump: restoring data for table "t"
>> pg_dump: dumping contents of table t
>> pg_dump: setting owner and privileges for TABLE t
>> pg_dump: setting owner and privileges for TABLE DATA t
>
> I don't see anything particularly incorrect about that. The point of
> the --verbose switch is to track what pg_dump is doing, and if what
> it's doing involves going through RestoreArchive(), why should we try
> to hide the fact?

"restoring data for table 't'" makes you think it's actuall restoring
things. It's not. That dumping is implemented by calling an internal
function called RestoreArchive() has to be an implementation detail...
It certainly confuses users that we say "restoring" when we're not
doing that...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I don't see anything particularly incorrect about that. The point of
>> the --verbose switch is to track what pg_dump is doing, and if what
>> it's doing involves going through RestoreArchive(), why should we try
>> to hide the fact?

> "restoring data for table 't'" makes you think it's actuall restoring
> things. It's not. That dumping is implemented by calling an internal
> function called RestoreArchive() has to be an implementation detail...
> It certainly confuses users that we say "restoring" when we're not
> doing that...

Well, why don't we just s/restoring/processing/ in the debug message,
and call it good?

regards, tom lane

On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I don't see anything particularly incorrect about that. The point of
> >> the --verbose switch is to track what pg_dump is doing, and if what
> >> it's doing involves going through RestoreArchive(), why should we try
> >> to hide the fact?
>
> > "restoring data for table 't'" makes you think it's actuall restoring
> > things. It's not. That dumping is implemented by calling an internal
> > function called RestoreArchive() has to be an implementation detail...
> > It certainly confuses users that we say "restoring" when we're not
> > doing that...
>
> Well, why don't we just s/restoring/processing/ in the debug message,
> and call it good?

Sure, that would work for me... I can go do that if there are no objections.

/Magnus

On Fri, Aug 31, 2012 at 2:05 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>
> On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> > On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> >> I don't see anything particularly incorrect about that. The point of
>> >> the --verbose switch is to track what pg_dump is doing, and if what
>> >> it's doing involves going through RestoreArchive(), why should we try
>> >> to hide the fact?
>>
>> > "restoring data for table 't'" makes you think it's actuall restoring
>> > things. It's not. That dumping is implemented by calling an internal
>> > function called RestoreArchive() has to be an implementation detail...
>> > It certainly confuses users that we say "restoring" when we're not
>> > doing that...
>>
>> Well, why don't we just s/restoring/processing/ in the debug message,
>> and call it good?
>
> Sure, that would work for me... I can go do that if there are no objections.

Done.

Are we "allowed" to backpatch things to 9.2 at this point that changes
strings for translators?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Well, why don't we just s/restoring/processing/ in the debug message,
>>> and call it good?

> Are we "allowed" to backpatch things to 9.2 at this point that changes
> strings for translators?

Well, not being a translator I'm not sure that I get a vote.
But I'd think this is too minor to justify back-patching it.

regards, tom lane

On Tue, Sep 4, 2012 at 3:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>> On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Well, why don't we just s/restoring/processing/ in the debug message,
>>>> and call it good?
>
>> Are we "allowed" to backpatch things to 9.2 at this point that changes
>> strings for translators?
>
> Well, not being a translator I'm not sure that I get a vote.
> But I'd think this is too minor to justify back-patching it.

Probably - that's why I didn't even consider going back beyond 9.2.

Anyway; I'll just leave it at master.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/