On Wed, Aug 20, 2014 at 2:43 AM, Michael Paquier <[email protected]>
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
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 3aebac8..07cc10e 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -546,8 +546,13 @@ RestoreArchive(Archive *AHX)
/* Both schema and data objects might now have ownership/ACLs */
if ((te->reqs & (REQ_SCHEMA | REQ_DATA)) != 0)
{
- ahlog(AH, 1, "setting owner and privileges for %s %s\n",
- te->desc, te->tag);
+ /* Show namespace if available */
+ if (te->namespace)
+ ahlog(AH, 1, "setting owner and privileges for %s \"%s\".\"%s\"\n",
+ te->desc, te->namespace, te->tag);
+ else
+ ahlog(AH, 1, "setting owner and privileges for %s \"%s\"\n",
+ te->desc, te->tag);
_printTocEntry(AH, te, ropt, false, true);
}
}
@@ -621,7 +626,13 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te,
if ((reqs & REQ_SCHEMA) != 0) /* We want the schema */
{
- ahlog(AH, 1, "creating %s %s\n", te->desc, te->tag);
+ /* Show namespace if available */
+ if (te->namespace)
+ ahlog(AH, 1, "creating %s \"%s\".\"%s\"\n",
+ te->desc, te->namespace, te->tag);
+ else
+ ahlog(AH, 1, "creating %s \"%s\"\n", te->desc, te->tag);
+
_printTocEntry(AH, te, ropt, false, false);
defnDumped = true;
@@ -713,8 +724,13 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te,
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
- ahlog(AH, 1, "processing data for table \"%s\"\n",
- te->tag);
+ /* Show namespace if available */
+ if (te->namespace)
+ ahlog(AH, 1, "processing data for table \"%s\".\"%s\"\n",
+ te->namespace, te->tag);
+ else
+ ahlog(AH, 1, "processing data for table \"%s\"\n",
+ te->tag);
/*
* In parallel restore, if we created the table earlier in
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 5c0f95f..ea32b42 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1383,6 +1383,8 @@ dumpTableData_copy(Archive *fout, void *dcontext)
{
TableDataInfo *tdinfo = (TableDataInfo *) dcontext;
TableInfo *tbinfo = tdinfo->tdtable;
+ const char *namespace = tbinfo->dobj.namespace ?
+ tbinfo->dobj.namespace->dobj.name : NULL;
const char *classname = tbinfo->dobj.name;
const bool hasoids = tbinfo->hasoids;
const bool oids = tdinfo->oids;
@@ -1400,7 +1402,16 @@ dumpTableData_copy(Archive *fout, void *dcontext)
const char *column_list;
if (g_verbose)
- write_msg(NULL, "dumping contents of table %s\n", classname);
+ {
+ /* Print namespace information if available */
+ if (namespace)
+ write_msg(NULL, "dumping contents of table \"%s\".\"%s\"\n",
+ namespace,
+ classname);
+ else
+ write_msg(NULL, "dumping contents of table \"%s\"\n",
+ classname);
+ }
/*
* Make sure we are in proper schema. We will qualify the table name
@@ -5019,8 +5030,16 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
continue;
if (g_verbose)
- write_msg(NULL, "reading indexes for table \"%s\"\n",
- tbinfo->dobj.name);
+ {
+ /* Print namespace information if available */
+ if (tbinfo->dobj.namespace)
+ write_msg(NULL, "reading indexes for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
+ else
+ write_msg(NULL, "reading indexes for table \"%s\"\n",
+ tbinfo->dobj.name);
+ }
/* Make sure we are in proper schema so indexdef is right */
selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
@@ -5385,8 +5404,16 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
continue;
if (g_verbose)
- write_msg(NULL, "reading foreign key constraints for table \"%s\"\n",
- tbinfo->dobj.name);
+ {
+ /* Print namespace information if available */
+ if (tbinfo->dobj.namespace)
+ write_msg(NULL, "reading foreign key constraints for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
+ else
+ write_msg(NULL, "reading foreign key constraints for table \"%s\"\n",
+ tbinfo->dobj.name);
+ }
/*
* select table schema to ensure constraint expr is qualified if
@@ -5723,8 +5750,16 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
continue;
if (g_verbose)
- write_msg(NULL, "reading triggers for table \"%s\"\n",
- tbinfo->dobj.name);
+ {
+ /* Print namespace information if available */
+ if (tbinfo->dobj.namespace)
+ write_msg(NULL, "reading triggers for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
+ else
+ write_msg(NULL, "reading triggers for table \"%s\"\n",
+ tbinfo->dobj.name);
+ }
/*
* select table schema to ensure regproc name is qualified if needed
@@ -6336,8 +6371,16 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
* the output of an indexscan on pg_attribute_relid_attnum_index.
*/
if (g_verbose)
- write_msg(NULL, "finding the columns and types of table \"%s\"\n",
- tbinfo->dobj.name);
+ {
+ /* Print namespace information if available */
+ if (tbinfo->dobj.namespace)
+ write_msg(NULL, "finding the columns and types of table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
+ else
+ write_msg(NULL, "finding the columns and types of table \"%s\"\n",
+ tbinfo->dobj.name);
+ }
resetPQExpBuffer(q);
@@ -6548,8 +6591,16 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
int numDefaults;
if (g_verbose)
- write_msg(NULL, "finding default expressions of table \"%s\"\n",
- tbinfo->dobj.name);
+ {
+ /* Print namespace information if available */
+ if (tbinfo->dobj.namespace)
+ write_msg(NULL, "finding default expressions of table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
+ else
+ write_msg(NULL, "finding default expressions of table \"%s\"\n",
+ tbinfo->dobj.name);
+ }
resetPQExpBuffer(q);
if (fout->remoteVersion >= 70300)
@@ -6672,8 +6723,16 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
int numConstrs;
if (g_verbose)
- write_msg(NULL, "finding check constraints for table \"%s\"\n",
- tbinfo->dobj.name);
+ {
+ /* Print namespace information if available */
+ if (tbinfo->dobj.namespace)
+ write_msg(NULL, "finding check constraints for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
+ else
+ write_msg(NULL, "finding check constraints for table \"%s\"\n",
+ tbinfo->dobj.name);
+ }
resetPQExpBuffer(q);
if (fout->remoteVersion >= 90200)
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
