pg_dump incorrect output in plaintext mode

Started by Magnus Haganderover 13 years ago9 messages
#1Magnus Hagander
magnus@hagander.net

$ 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/

#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Magnus Hagander (#1)
Re: pg_dump incorrect output in plaintext mode

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#1)
Re: pg_dump incorrect output in plaintext mode

Magnus Hagander <magnus@hagander.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

#4Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#3)
Re: pg_dump incorrect output in plaintext mode

On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.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/

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#4)
Re: pg_dump incorrect output in plaintext mode

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl@sss.pgh.pa.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

#6Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#5)
Re: pg_dump incorrect output in plaintext mode

On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl@sss.pgh.pa.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

#7Magnus Hagander
magnus@hagander.net
In reply to: Magnus Hagander (#6)
Re: pg_dump incorrect output in plaintext mode

On Fri, Aug 31, 2012 at 2:05 PM, Magnus Hagander <magnus@hagander.net> wrote:

On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Tue, Aug 28, 2012 at 6:42 PM, Tom Lane <tgl@sss.pgh.pa.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/

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#7)
Re: pg_dump incorrect output in plaintext mode

Magnus Hagander <magnus@hagander.net> writes:

On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl@sss.pgh.pa.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

#9Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#8)
Re: pg_dump incorrect output in plaintext mode

On Tue, Sep 4, 2012 at 3:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Aug 28, 2012 9:59 PM, "Tom Lane" <tgl@sss.pgh.pa.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/