From: | Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Verbose output of pg_dump not show schema name |
Date: | 2014-08-20 20:11:42 |
Message-ID: | CAFcNs+qQiyQHSsSzrM0U9t+e5cAS8q98_P4nOYxGMOK1c=JH-w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Aug 20, 2014 at 2:43 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:
>
> I had a look at this patch, and here are a couple of comments:
> 1) Depending on how ArchiveEntry is called to register an object to
> dump, namespace may be NULL, but it is not the case
> namespace->dobj.name, so you could get the namespace name at the top
> of the function that have their verbose output improved with something
> like that:
> const char *namespace = tbinfo->dobj.namespace ?
> tbinfo->dobj.namespace->dobj.name : NULL;
> And then simplify the message output as follows:
> if (namespace)
> write_msg("blah \"%s\".\"%s\" blah", namespace, classname);
> else
> write_msg("blah \"%s\" blah", classname);
> You can as well safely remove the checks on namespace->dobj.name.
Ok
> 2) I don't think that this is correct:
> - ahlog(AH, 1, "processing data
> for table \"%s\"\n",
> - te->tag);
> + ahlog(AH, 1, "processing data
> for table \"%s\".\"%s\"\n",
> + AH->currSchema,
te->tag);
> There are some code paths where AH->currSchema is set to NULL, and I
> think that you should use te->namespace instead.
Yes, you are correct!
> 3) Changing only this message is not enough. The following verbose
> messages need to be changed too for consistency:
> - pg_dump: creating $tag $object
> - pg_dump: setting owner and privileges for [blah]
>
> I have been pondering as well about doing similar modifications to the
> error message paths, but it did not seem worth it as this patch is
> aimed only for the verbose output. Btw, I have basically fixed those
> issues while doing the review, and finished with the attached patch.
> Fabrizio, is this new version fine for you?
>
Is fine to me.
I just change "if (tbinfo->dobj.namespace != NULL)" to "if
(tbinfo->dobj.namespace)".
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
Attachment | Content-Type | Size |
---|---|---|
dump_restore_schema_verbose_v2.patch | text/x-diff | 6.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andreas 'ads' Scherbaum | 2014-08-20 20:13:17 | documentation update for doc/src/sgml/func.sgml |
Previous Message | Mayeul Kauffmann | 2014-08-20 19:34:31 | Re: Patching for increasing the number of columns |