On Fri, Jul 25, 2014 at 4:45 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
>
> Given this is a very small and simple patch I thought it's not necessary...
>
> Added to the next CommitFest.
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.
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.
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?
Regards,
--
Michael
