pg_upgrade - typo in verbose log

Started by Peter Smithover 2 years ago7 messages
#1Peter Smith
smithpb2250@gmail.com
1 attachment(s)

Hi --

While reviewing another patch for the file info.c, I noticed some
misplaced colon (':') in the verbose logs for print_rel_infos().

PSA patch to fix it.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v1-0001-pg_upgrade-typo-in-verbose-log.patchapplication/octet-stream; name=v1-0001-pg_upgrade-typo-in-verbose-log.patchDownload
From bfe9f714dde7c7e515b9abcd56ce96d408537e5f Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Thu, 11 May 2023 15:30:42 +1000
Subject: [PATCH v1] pg_upgrade - typo in verbose log

---
 src/bin/pg_upgrade/info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index bf3244f..f02aaaf 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -654,7 +654,7 @@ print_rel_infos(RelInfoArr *rel_arr)
 	int			relnum;
 
 	for (relnum = 0; relnum < rel_arr->nrels; relnum++)
-		pg_log(PG_VERBOSE, "relname: %s.%s: reloid: %u reltblspace: %s",
+		pg_log(PG_VERBOSE, "relname: %s.%s, reloid: %u, reltblspace: %s",
 			   rel_arr->rels[relnum].nspname,
 			   rel_arr->rels[relnum].relname,
 			   rel_arr->rels[relnum].reloid,
-- 
1.8.3.1

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Smith (#1)
Re: pg_upgrade - typo in verbose log

On 11 May 2023, at 07:41, Peter Smith <smithpb2250@gmail.com> wrote:

While reviewing another patch for the file info.c, I noticed some
misplaced colon (':') in the verbose logs for print_rel_infos().

That spelling was introduced in c2e9b2f28818 which was the initial import of
pg_upgrade into contrib/ for the 9.0 release (at that time the function was
relarr_print() which via a few other names was renamed to print_rel_infos() in
0a5f1199319).

It's not entirely clear to me if the current spelling is a mistake or
intentional, but I do agree that your version is an improvement.

To be consistent with other log output in pg_upgrade we should probably also
wrap the relname and reltblspace in quotes as \"%s.%s\" and \"%s\" (and the
Database in print_db_infos()).

--
Daniel Gustafsson

#3Peter Smith
smithpb2250@gmail.com
In reply to: Daniel Gustafsson (#2)
1 attachment(s)
Re: pg_upgrade - typo in verbose log

On Thu, May 11, 2023 at 6:30 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 11 May 2023, at 07:41, Peter Smith <smithpb2250@gmail.com> wrote:

While reviewing another patch for the file info.c, I noticed some
misplaced colon (':') in the verbose logs for print_rel_infos().

That spelling was introduced in c2e9b2f28818 which was the initial import of
pg_upgrade into contrib/ for the 9.0 release (at that time the function was
relarr_print() which via a few other names was renamed to print_rel_infos() in
0a5f1199319).

It's not entirely clear to me if the current spelling is a mistake or
intentional, but I do agree that your version is an improvement.

To be consistent with other log output in pg_upgrade we should probably also
wrap the relname and reltblspace in quotes as \"%s.%s\" and \"%s\" (and the
Database in print_db_infos()).

Thanks for checking, and for the feedback.

PSA patch v2 updated as suggested.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v2-0001-pg_upgrade-typo-in-verbose-log.patchapplication/octet-stream; name=v2-0001-pg_upgrade-typo-in-verbose-log.patchDownload
From 32466e00816ecf298af0e733b7a919127acc4c36 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Thu, 11 May 2023 19:05:17 +1000
Subject: [PATCH v2] pg_upgrade - typo in verbose log

---
 src/bin/pg_upgrade/info.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index bf3244f..64b6d39 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -642,7 +642,7 @@ print_db_infos(DbInfoArr *db_arr)
 
 	for (dbnum = 0; dbnum < db_arr->ndbs; dbnum++)
 	{
-		pg_log(PG_VERBOSE, "Database: %s", db_arr->dbs[dbnum].db_name);
+		pg_log(PG_VERBOSE, "Database: \"%s\"", db_arr->dbs[dbnum].db_name);
 		print_rel_infos(&db_arr->dbs[dbnum].rel_arr);
 	}
 }
@@ -654,7 +654,7 @@ print_rel_infos(RelInfoArr *rel_arr)
 	int			relnum;
 
 	for (relnum = 0; relnum < rel_arr->nrels; relnum++)
-		pg_log(PG_VERBOSE, "relname: %s.%s: reloid: %u reltblspace: %s",
+		pg_log(PG_VERBOSE, "relname: \"%s.%s\", reloid: %u, reltblspace: \"%s\"",
 			   rel_arr->rels[relnum].nspname,
 			   rel_arr->rels[relnum].relname,
 			   rel_arr->rels[relnum].reloid,
-- 
1.8.3.1

#4Peter Smith
smithpb2250@gmail.com
In reply to: Peter Smith (#3)
Re: pg_upgrade - typo in verbose log

On Thu, May 11, 2023 at 7:09 PM Peter Smith <smithpb2250@gmail.com> wrote:

On Thu, May 11, 2023 at 6:30 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 11 May 2023, at 07:41, Peter Smith <smithpb2250@gmail.com> wrote:

While reviewing another patch for the file info.c, I noticed some
misplaced colon (':') in the verbose logs for print_rel_infos().

That spelling was introduced in c2e9b2f28818 which was the initial import of
pg_upgrade into contrib/ for the 9.0 release (at that time the function was
relarr_print() which via a few other names was renamed to print_rel_infos() in
0a5f1199319).

It's not entirely clear to me if the current spelling is a mistake or
intentional, but I do agree that your version is an improvement.

To be consistent with other log output in pg_upgrade we should probably also
wrap the relname and reltblspace in quotes as \"%s.%s\" and \"%s\" (and the
Database in print_db_infos()).

Thanks for checking, and for the feedback.

PSA patch v2 updated as suggested.

Ping.

I thought v2 was ready to be pushed, but then this thread went silent
for 3 months

??

------
Kind Regards,
Peter Smith.
Fujitsu Australia

#5Michael Paquier
michael@paquier.xyz
In reply to: Peter Smith (#4)
Re: pg_upgrade - typo in verbose log

On Thu, Aug 17, 2023 at 06:09:31PM +1000, Peter Smith wrote:

Ping.

Funnily enough, I was looking at this entry yesterday, before you
replied, and was wondering what's happening here.

I thought v2 was ready to be pushed, but then this thread went silent
for 3 months

Change looks fine, so applied.
--
Michael

#6Peter Smith
smithpb2250@gmail.com
In reply to: Michael Paquier (#5)
Re: pg_upgrade - typo in verbose log

On Fri, Aug 18, 2023 at 10:47 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Aug 17, 2023 at 06:09:31PM +1000, Peter Smith wrote:

Ping.

Funnily enough, I was looking at this entry yesterday, before you
replied, and was wondering what's happening here.

I thought v2 was ready to be pushed, but then this thread went silent
for 3 months

Change looks fine, so applied.

Thanks!

------
Kind Regards,
Peter Smith.
Fujitsu Australia

#7Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#5)
Re: pg_upgrade - typo in verbose log

On 18 Aug 2023, at 02:47, Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Aug 17, 2023 at 06:09:31PM +1000, Peter Smith wrote:

Ping.

Funnily enough, I was looking at this entry yesterday, before you
replied, and was wondering what's happening here.

It was a combination of summer vacation, doing CFM and looking at things for
v16, and some things were left further down on the TODO stack duing this.

I thought v2 was ready to be pushed, but then this thread went silent
for 3 months

Change looks fine, so applied.

Agreed, and thanks for applying.

--
Daniel Gustafsson