Verbose output of pg_dump not show schema name
Hi all,
There are some reason to verbose output of pg_dump don't show schema name?
A output example of using "pg_dump -Fd -j8 -v"
...
pg_dump: dumping contents of table geocoordinate
pg_dump: dumping contents of table historyvalue
pg_dump: dumping contents of table historyvalue
pg_dump: dumping contents of table transactionlog
pg_dump: dumping contents of table historyvalue
pg_dump: dumping contents of table historyvalue
pg_dump: dumping contents of table historyvalue
pg_dump: dumping contents of table historyvalue
...
This database have a lot of different schemas with same structure and if I
need do view the status of dump I don't know what schema the table are dump
from.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
On Thu, Apr 17, 2014 at 11:41 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
Hi all,
There are some reason to verbose output of pg_dump don't show schema name?
A output example of using "pg_dump -Fd -j8 -v"
Specifying a target directory with "-f" is better here...
This database have a lot of different schemas with same structure and if I
need do view the status of dump I don't know what schema the table are dump
from.
Yes this may be helpful. The attached quick'n dirty patch implements it.
--
Michael
Attachments:
20140417_dump_schema_verbose.patchtext/plain; charset=US-ASCII; name=20140417_dump_schema_verbose.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a6c0428..65c3cd4 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1400,7 +1400,17 @@ 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 (tbinfo->dobj.namespace != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ write_msg(NULL, "dumping contents of table %s.%s\n",
+ tbinfo->dobj.namespace->dobj.name,
+ 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
On Thu, Apr 17, 2014 at 2:14 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Thu, Apr 17, 2014 at 11:41 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:Hi all,
There are some reason to verbose output of pg_dump don't show schema
name?
A output example of using "pg_dump -Fd -j8 -v"
Specifying a target directory with "-f" is better here...
Yeah... I'm just show the relevant options used... ;-)
This database have a lot of different schemas with same structure and
if I
need do view the status of dump I don't know what schema the table are
dump
from.
Yes this may be helpful. The attached quick'n dirty patch implements it.
Very nice... thanks!!!
I add schema name do the following messages too:
pg_restore: processing data for table "public"."bar"
pg_restore: processing data for table "public"."foo"
pg_restore: processing data for table "s1"."bar"
pg_restore: processing data for table "s1"."foo"
pg_restore: processing data for table "s2"."bar"
pg_restore: processing data for table "s2"."foo"
pg_restore: processing data for table "s3"."bar"
pg_restore: processing data for table "s3"."foo"
And:
pg_dump: finding the columns and types of table "s1"."foo"
pg_dump: finding the columns and types of table "s1"."bar"
pg_dump: finding the columns and types of table "s2"."foo"
pg_dump: finding the columns and types of table "s2"."bar"
pg_dump: finding the columns and types of table "s3"."foo"
pg_dump: finding the columns and types of table "s3"."bar"
pg_dump: finding the columns and types of table "public"."foo"
pg_dump: finding the columns and types of table "public"."bar"
And:
pg_dump: processing data for table "public"."bar"
pg_dump: dumping contents of table public.bar
pg_dump: processing data for table "public"."foo"
pg_dump: dumping contents of table public.foo
pg_dump: processing data for table "s1"."bar"
pg_dump: dumping contents of table s1.bar
pg_dump: processing data for table "s1"."foo"
pg_dump: dumping contents of table s1.foo
pg_dump: processing data for table "s2"."bar"
pg_dump: dumping contents of table s2.bar
pg_dump: processing data for table "s2"."foo"
pg_dump: dumping contents of table s2.foo
pg_dump: processing data for table "s3"."bar"
pg_dump: dumping contents of table s3.bar
pg_dump: processing data for table "s3"."foo"
pg_dump: dumping contents of table s3.foo
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Attachments:
dump_restore_schema_verbose_v1.patchtext/x-diff; charset=US-ASCII; name=dump_restore_schema_verbose_v1.patchDownload
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 9464540..7f73e8d 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -706,8 +706,8 @@ 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);
+ ahlog(AH, 1, "processing data for table \"%s\".\"%s\"\n",
+ AH->currSchema, 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 a6c0428..f763a88 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1400,7 +1400,17 @@ 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 (tbinfo->dobj.namespace != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ write_msg(NULL, "dumping contents of table %s.%s\n",
+ tbinfo->dobj.namespace->dobj.name,
+ 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
@@ -6291,8 +6301,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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);
On Thu, Apr 17, 2014 at 11:29:03AM -0300, Fabr�zio de Royes Mello wrote:
This database have a lot of different schemas with same structure and if I
need do view the status of dump I don't know what schema the table are dump
from.Yes this may be helpful. The attached quick'n dirty patch implements it.
Very nice... thanks!!!
I add schema name do the following messages too:
pg_restore: processing data for table "public"."bar"
pg_restore: processing data for table "public"."foo"
pg_restore: processing data for table "s1"."bar"
pg_restore: processing data for table "s1"."foo"
pg_restore: processing data for table "s2"."bar"
pg_restore: processing data for table "s2"."foo"
pg_restore: processing data for table "s3"."bar"
pg_restore: processing data for table "s3"."foo"
Can you get that to _conditionally_ double-quote the strings? In fact,
maybe we don't even need the double-quotes. How do we double-quote
other places?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 17, 2014 at 11:36 AM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 17, 2014 at 11:29:03AM -0300, Fabrízio de Royes Mello wrote:
This database have a lot of different schemas with same structure
and if I
need do view the status of dump I don't know what schema the table
are dump
from.
Yes this may be helpful. The attached quick'n dirty patch implements
it.
Very nice... thanks!!!
I add schema name do the following messages too:
pg_restore: processing data for table "public"."bar"
pg_restore: processing data for table "public"."foo"
pg_restore: processing data for table "s1"."bar"
pg_restore: processing data for table "s1"."foo"
pg_restore: processing data for table "s2"."bar"
pg_restore: processing data for table "s2"."foo"
pg_restore: processing data for table "s3"."bar"
pg_restore: processing data for table "s3"."foo"Can you get that to _conditionally_ double-quote the strings?
Sorry, I didn't understand what you means? Your idea is to check if the
namespace is available and then don't show the double-quote, is that?
In fact,
maybe we don't even need the double-quotes. How do we double-quote
other places?
Checking that more deeply I found some other places that show the table
name and all of them are double-quoted.
$ grep 'table \\\"%s' src/bin/pg_dump/*.c
src/bin/pg_dump/common.c: write_msg(NULL, "failed sanity
check, parent OID %u of table \"%s\" (OID %u) not found\n",
src/bin/pg_dump/pg_backup_archiver.c: ahlog(AH, 1,
"processing data for table \"%s\".\"%s\"\n",
src/bin/pg_dump/pg_backup_archiver.c: ahlog(AH, 1, "table \"%s\" could
not be created, will not restore its data\n",
src/bin/pg_dump/pg_backup_db.c: warn_or_exit_horribly(AH,
modulename, "COPY failed for table \"%s\": %s",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "Dumping the contents of
table \"%s\" failed: PQgetCopyData() failed.\n", classname);
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "Dumping the contents of
table \"%s\" failed: PQgetResult() failed.\n", classname);
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "WARNING: owner of
table \"%s\" appears to be invalid\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "reading indexes for
table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "reading foreign key
constraints for table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "reading triggers for
table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: exit_horribly(NULL,
"query produced null referenced table name for foreign key trigger \"%s\"
on table \"%s\" (OID of table: %u)\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "finding the
columns and types of table \"%s\".\"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "finding the
columns and types of table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: "invalid column
numbering in table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "finding default
expressions of table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: "invalid adnum
value %d for table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "finding check
constraints for table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL,
ngettext("expected %d check constraint on table \"%s\" but found %d\n",
src/bin/pg_dump/pg_dump.c:
"expected %d check constraints on table \"%s\" but found %d\n",
src/bin/pg_dump/pg_dump.c: exit_horribly(NULL, "invalid column number %d
for table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "invalid argument
string (%s) for trigger \"%s\" on table \"%s\"\n",
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "query to get rule \"%s\"
for table \"%s\" failed: wrong number of rows returned\n",
Just the "dumping contents of table.." message isn't double-quoted:
$ grep 'table %s' src/bin/pg_dump/*.c
src/bin/pg_dump/pg_dump.c: write_msg(NULL, "dumping contents of
table %s\n",
So maybe we must double-quote of all string, i.e. "public.foo", including
the missing bellow.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
On Thu, Apr 17, 2014 at 12:07:39PM -0300, Fabr�zio de Royes Mello wrote:
Can you get that to _conditionally_ double-quote the strings?�
Sorry, I didn't understand what you means? Your idea is to check if the
namespace is available and then don't show the double-quote, is that?
The idea is that we only need quotes when there are odd characters in
the identifier. We do that right now in some places, though I can't
find them in pg_dump. I know psql does that, see quote_ident().
In fact,
maybe we don't even need the double-quotes. �How do we double-quote
other places?Checking that more deeply I found some other places that show the table name
and all of them are double-quoted.
OK.
Just the "dumping contents of table.." message isn't double-quoted:
$ grep 'table %s' src/bin/pg_dump/*.c
src/bin/pg_dump/pg_dump.c:��� ��� ��� write_msg(NULL, "dumping contents of
table %s\n",So maybe we must double-quote of all string, i.e. "public.foo", including the
missing bellow.
No, I think double-quoting each part is the correct way.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Bruce Momjian <bruce@momjian.us> writes:
The idea is that we only need quotes when there are odd characters in
the identifier. We do that right now in some places, though I can't
find them in pg_dump. I know psql does that, see quote_ident().
I think our general style rule is that identifiers embedded in messages
are always double-quoted. There's an exception for type names, but
not otherwise. You're confusing the message case with printing SQL.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 17, 2014 at 11:44:37AM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
The idea is that we only need quotes when there are odd characters in
the identifier. We do that right now in some places, though I can't
find them in pg_dump. I know psql does that, see quote_ident().I think our general style rule is that identifiers embedded in messages
are always double-quoted. There's an exception for type names, but
not otherwise. You're confusing the message case with printing SQL.
OK. I was unclear if a status _display_ was a message like an error
message.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 17, 2014 at 12:46 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 17, 2014 at 11:44:37AM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
The idea is that we only need quotes when there are odd characters in
the identifier. We do that right now in some places, though I can't
find them in pg_dump. I know psql does that, see quote_ident().I think our general style rule is that identifiers embedded in messages
are always double-quoted. There's an exception for type names, but
not otherwise. You're confusing the message case with printing SQL.OK. I was unclear if a status _display_ was a message like an error
message.
The attached patch fix missing double-quoted in "dumping contents of
table.." message and add schema name to other messages:
- "reading indexes for table \"%s\".\"%s\"\n"
- "reading foreign key constraints for table \"%s\".\"%s\"\n"
- "reading triggers for table \"%s\".\"%s\"\n"
- "finding the columns and types of table \"%s\".\"%s\"\n"
- "finding default expressions of table \"%s\".\"%s\"\n"
- "finding check constraints for table \"%s\".\"%s\"\n"
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Attachments:
dump_restore_schema_verbose_v2.patchtext/x-diff; charset=US-ASCII; name=dump_restore_schema_verbose_v2.patchDownload
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 9464540..7f73e8d 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -706,8 +706,8 @@ 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);
+ ahlog(AH, 1, "processing data for table \"%s\".\"%s\"\n",
+ AH->currSchema, 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 a6c0428..78ec5bf 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1400,7 +1400,17 @@ 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 (tbinfo->dobj.namespace != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ write_msg(NULL, "dumping contents of table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ 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
@@ -4974,8 +4984,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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);
@@ -5340,8 +5359,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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
@@ -5678,8 +5706,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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
@@ -6291,8 +6328,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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);
@@ -6503,8 +6549,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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)
@@ -6627,8 +6682,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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)
On Fri, Apr 18, 2014 at 4:29 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
On Thu, Apr 17, 2014 at 12:46 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Thu, Apr 17, 2014 at 11:44:37AM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
The idea is that we only need quotes when there are odd characters in
the identifier. We do that right now in some places, though I can't
find them in pg_dump. I know psql does that, see quote_ident().I think our general style rule is that identifiers embedded in messages
are always double-quoted. There's an exception for type names, but
not otherwise. You're confusing the message case with printing SQL.OK. I was unclear if a status _display_ was a message like an error
message.The attached patch fix missing double-quoted in "dumping contents of
table.." message and add schema name to other messages:
- "reading indexes for table \"%s\".\"%s\"\n"
- "reading foreign key constraints for table \"%s\".\"%s\"\n"
- "reading triggers for table \"%s\".\"%s\"\n"- "finding the columns and types of table \"%s\".\"%s\"\n"
- "finding default expressions of table \"%s\".\"%s\"\n"
- "finding check constraints for table \"%s\".\"%s\"\n"
Cool additions. There may be a more elegant way to check if namespace
is NULL, but I couldn't come up with one myself. So patch may be fine.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 17, 2014 at 9:15 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Fri, Apr 18, 2014 at 4:29 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Apr 17, 2014 at 12:46 PM, Bruce Momjian <bruce@momjian.us>
wrote:
On Thu, Apr 17, 2014 at 11:44:37AM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
The idea is that we only need quotes when there are odd characters
in
the identifier. We do that right now in some places, though I
can't
find them in pg_dump. I know psql does that, see quote_ident().
I think our general style rule is that identifiers embedded in
messages
are always double-quoted. There's an exception for type names, but
not otherwise. You're confusing the message case with printing SQL.OK. I was unclear if a status _display_ was a message like an error
message.The attached patch fix missing double-quoted in "dumping contents of
table.." message and add schema name to other messages:
- "reading indexes for table \"%s\".\"%s\"\n"
- "reading foreign key constraints for table \"%s\".\"%s\"\n"
- "reading triggers for table \"%s\".\"%s\"\n"- "finding the columns and types of table \"%s\".\"%s\"\n"
- "finding default expressions of table \"%s\".\"%s\"\n"
- "finding check constraints for table \"%s\".\"%s\"\n"Cool additions. There may be a more elegant way to check if namespace
is NULL, but I couldn't come up with one myself. So patch may be fine.
Hi all,
I think this small patch was lost. There are something wrong?
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
On Wed, Jul 23, 2014 at 5:48 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
On Thu, Apr 17, 2014 at 9:15 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:On Fri, Apr 18, 2014 at 4:29 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Apr 17, 2014 at 12:46 PM, Bruce Momjian <bruce@momjian.us>
wrote:On Thu, Apr 17, 2014 at 11:44:37AM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
The idea is that we only need quotes when there are odd characters
in
the identifier. We do that right now in some places, though I
can't
find them in pg_dump. I know psql does that, see quote_ident().I think our general style rule is that identifiers embedded in
messages
are always double-quoted. There's an exception for type names, but
not otherwise. You're confusing the message case with printing SQL.OK. I was unclear if a status _display_ was a message like an error
message.The attached patch fix missing double-quoted in "dumping contents of
table.." message and add schema name to other messages:
- "reading indexes for table \"%s\".\"%s\"\n"
- "reading foreign key constraints for table \"%s\".\"%s\"\n"
- "reading triggers for table \"%s\".\"%s\"\n"- "finding the columns and types of table \"%s\".\"%s\"\n"
- "finding default expressions of table \"%s\".\"%s\"\n"
- "finding check constraints for table \"%s\".\"%s\"\n"Cool additions. There may be a more elegant way to check if namespace
is NULL, but I couldn't come up with one myself. So patch may be fine.Hi all,
I think this small patch was lost. There are something wrong?
Did it get added to a CommitFest?
I don't see it there.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jul 24, 2014 at 4:36 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Jul 23, 2014 at 5:48 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Apr 17, 2014 at 9:15 PM, Michael Paquier <
michael.paquier@gmail.com>
wrote:
On Fri, Apr 18, 2014 at 4:29 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:On Thu, Apr 17, 2014 at 12:46 PM, Bruce Momjian <bruce@momjian.us>
wrote:On Thu, Apr 17, 2014 at 11:44:37AM -0400, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
The idea is that we only need quotes when there are odd
characters
in
the identifier. We do that right now in some places, though I
can't
find them in pg_dump. I know psql does that, see quote_ident().I think our general style rule is that identifiers embedded in
messages
are always double-quoted. There's an exception for type names,
but
not otherwise. You're confusing the message case with printing
SQL.
OK. I was unclear if a status _display_ was a message like an error
message.The attached patch fix missing double-quoted in "dumping contents of
table.." message and add schema name to other messages:
- "reading indexes for table \"%s\".\"%s\"\n"
- "reading foreign key constraints for table \"%s\".\"%s\"\n"
- "reading triggers for table \"%s\".\"%s\"\n"- "finding the columns and types of table \"%s\".\"%s\"\n"
- "finding default expressions of table \"%s\".\"%s\"\n"
- "finding check constraints for table \"%s\".\"%s\"\n"Cool additions. There may be a more elegant way to check if namespace
is NULL, but I couldn't come up with one myself. So patch may be fine.Hi all,
I think this small patch was lost. There are something wrong?
Did it get added to a CommitFest?
I don't see it there.
Given this is a very small and simple patch I thought it's not necessary...
Added to the next CommitFest.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Attachments:
dump_restore_schema_verbose_v1.patchtext/x-diff; charset=US-ASCII; name=dump_restore_schema_verbose_v1.patchDownload
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 3aebac8..5c34a63 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -713,8 +713,8 @@ 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);
+ ahlog(AH, 1, "processing data for table \"%s\".\"%s\"\n",
+ AH->currSchema, 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 412f52f..3c7e88f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1400,7 +1400,17 @@ 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 (tbinfo->dobj.namespace != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ write_msg(NULL, "dumping contents of table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ 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 +5029,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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 +5751,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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 +6373,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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 +6594,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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 +6727,17 @@ 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 != NULL &&
+ tbinfo->dobj.namespace->dobj.name != NULL)
+ 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)
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
Attachments:
20140820_pgdump_verbose_nspace.patchtext/x-patch; charset=US-ASCII; name=20140820_pgdump_verbose_nspace.patchDownload
From e0809869655c9e22cce11955c7286cef8a42bf1d Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Wed, 20 Aug 2014 14:40:40 +0900
Subject: [PATCH] Improve verbose messages of pg_dump with namespace
Namespace is added to the verbose output when it is available, relation
and namespace names are put within quotes for clarity and consistency
with the other tools as well.
---
src/bin/pg_dump/pg_backup_archiver.c | 26 ++++++++---
src/bin/pg_dump/pg_dump.c | 85 ++++++++++++++++++++++++++++++------
2 files changed, 93 insertions(+), 18 deletions(-)
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..dd7eef9 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 != NULL)
+ 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 != NULL)
+ 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 != NULL)
+ 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 != NULL)
+ 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 != NULL)
+ 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 != NULL)
+ 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)
--
2.1.0
On Wed, Aug 20, 2014 at 2:43 AM, Michael Paquier <michael.paquier@gmail.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
Show quoted text
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
Attachments:
dump_restore_schema_verbose_v2.patchtext/x-diff; charset=US-ASCII; name=dump_restore_schema_verbose_v2.patchDownload
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)
On Thu, Aug 21, 2014 at 5:11 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
I just change "if (tbinfo->dobj.namespace != NULL)" to "if
(tbinfo->dobj.namespace)".
Fine for me. I am marking this patch as ready for committer.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Aug 20, 2014 at 8:24 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Thu, Aug 21, 2014 at 5:11 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:I just change "if (tbinfo->dobj.namespace != NULL)" to "if
(tbinfo->dobj.namespace)".Fine for me. I am marking this patch as ready for committer.
Thanks!
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
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
On 08/20/2014 11:11 PM, Fabr�zio de Royes Mello wrote:
On Wed, Aug 20, 2014 at 2:43 AM, Michael Paquier <michael.paquier@gmail.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
AFAICS, the namespace can never be NULL in any of these. There is a
"selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name)" call
before or after printing the message, so if tbinfo->dobj.namespace is
NULL, you'll crash anyway. Please double-check, and remove the dead code
if you agree.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Aug 26, 2014 at 3:48 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
AFAICS, the namespace can never be NULL in any of these. There is a
"selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name)" call before or
after printing the message, so if tbinfo->dobj.namespace is NULL, you'll
crash anyway. Please double-check, and remove the dead code if you agree.
Ah right, this field is used in many places. Even for
pg_backup_archiver.c, the portion of code processing data always has
the namespace set. I am sure that Fabrizio would have done that
quickly, but as I was on this thread I simplified the patch as
attached.
Regards,
--
Michael
Attachments:
20140826_pgdump_verbose_nspace_v3.patchtext/x-patch; charset=US-ASCII; name=20140826_pgdump_verbose_nspace_v3.patchDownload
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 3aebac8..7c0616d 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,9 @@ 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 */
+ ahlog(AH, 1, "processing data for table \"%s\".\"%s\"\n",
+ te->namespace, 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..c084ee9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1400,7 +1400,8 @@ dumpTableData_copy(Archive *fout, void *dcontext)
const char *column_list;
if (g_verbose)
- write_msg(NULL, "dumping contents of table %s\n", classname);
+ write_msg(NULL, "dumping contents of table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name, classname);
/*
* Make sure we are in proper schema. We will qualify the table name
@@ -5019,7 +5020,8 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
continue;
if (g_verbose)
- write_msg(NULL, "reading indexes for table \"%s\"\n",
+ write_msg(NULL, "reading indexes for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
tbinfo->dobj.name);
/* Make sure we are in proper schema so indexdef is right */
@@ -5385,7 +5387,8 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
continue;
if (g_verbose)
- write_msg(NULL, "reading foreign key constraints for table \"%s\"\n",
+ write_msg(NULL, "reading foreign key constraints for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
tbinfo->dobj.name);
/*
@@ -5723,7 +5726,8 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
continue;
if (g_verbose)
- write_msg(NULL, "reading triggers for table \"%s\"\n",
+ write_msg(NULL, "reading triggers for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
tbinfo->dobj.name);
/*
@@ -6336,8 +6340,9 @@ 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);
+ write_msg(NULL, "finding the columns and types of table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
resetPQExpBuffer(q);
@@ -6548,8 +6553,9 @@ 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);
+ write_msg(NULL, "finding default expressions of table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
+ tbinfo->dobj.name);
resetPQExpBuffer(q);
if (fout->remoteVersion >= 70300)
@@ -6672,7 +6678,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
int numConstrs;
if (g_verbose)
- write_msg(NULL, "finding check constraints for table \"%s\"\n",
+ write_msg(NULL, "finding check constraints for table \"%s\".\"%s\"\n",
+ tbinfo->dobj.namespace->dobj.name,
tbinfo->dobj.name);
resetPQExpBuffer(q);
On 08/26/2014 10:10 AM, Michael Paquier wrote:
On Tue, Aug 26, 2014 at 3:48 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:AFAICS, the namespace can never be NULL in any of these. There is a
"selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name)" call before or
after printing the message, so if tbinfo->dobj.namespace is NULL, you'll
crash anyway. Please double-check, and remove the dead code if you agree.Ah right, this field is used in many places. Even for
pg_backup_archiver.c, the portion of code processing data always has
the namespace set. I am sure that Fabrizio would have done that
quickly, but as I was on this thread I simplified the patch as
Ok thanks, committed.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Aug 26, 2014 at 4:10 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Tue, Aug 26, 2014 at 3:48 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:AFAICS, the namespace can never be NULL in any of these. There is a
"selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name)" call
before or
after printing the message, so if tbinfo->dobj.namespace is NULL, you'll
crash anyway. Please double-check, and remove the dead code if you
agree.
Ah right, this field is used in many places. Even for
pg_backup_archiver.c, the portion of code processing data always has
the namespace set. I am sure that Fabrizio would have done that
quickly, but as I was on this thread I simplified the patch as
attached.
Thanks Michael... last night I was working on a refactoring in
"tablecmds.c".
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Show quoted text
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