From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
Cc: | "Wei Wang (Fujitsu)" <wangw(dot)fnst(at)fujitsu(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Subject: | Re: [PoC] pg_upgrade: allow to upgrade publisher node |
Date: | 2023-05-15 07:39:57 |
Message-ID: | CAHut+PszSD_uctkL5U9kjt5BTurSY1o6sGHB3CH871VqKP7XMg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Kuroda-san.
I looked at the latest patch v13-0001. Here are some minor comments.
======
src/bin/pg_upgrade/info.c
1. get_logical_slot_infos_per_db
I noticed that the way this is coded, 'ntups' and 'num_slots' seems to
have exactly the same meaning. IMO you can simplify this by removing
'ntups'.
BEFORE
+ int ntups;
+ int num_slots = 0;
SUGGESTION
+ int num_slots;
~
BEFORE
+ ntups = PQntuples(res);
+
+ if (ntups)
+ {
SUGGESTION
+ num_slots = PQntuples(res);
+
+ if (num_slots)
+ {
~
BEFORE
+ slotinfos = (LogicalSlotInfo *) pg_malloc(sizeof(LogicalSlotInfo) * ntups);
SUGGESTION
+ slotinfos = (LogicalSlotInfo *) pg_malloc(sizeof(LogicalSlotInfo) *
num_slots);
~
BEFORE
+ for (slotnum = 0; slotnum < ntups; slotnum++)
+ {
+ LogicalSlotInfo *curr = &slotinfos[num_slots++];
SUGGESTION
+ for (slotnum = 0; slotnum < ntups; slotnum++)
+ {
+ LogicalSlotInfo *curr = &slotinfos[slotnum];
======
2. get_logical_slot_infos, print_slot_infos
> >
> > In another thread [1] I am posting some minor patch changes to the
> > VERBOSE logging (changes to double-quotes and commas etc.). Please
> > keep a watch on that thread because if gets pushed then this one will
> > be impacted. e.g. your logging here ought also to include the same
> > suggested double quotes.
>
> I thought it would be pushed soon, so the suggestion was included.
OK, but I think you have accidentally missed adding similar new double
quotes to all other VERBOSE logging in your patch.
e.g. see get_logical_slot_infos:
pg_log(PG_VERBOSE, "Database: %s", pDbInfo->db_name);
------
Kind Regards,
Peter Smith.
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-05-15 07:45:39 | Re: Redundant strlen(query) in get_rel_infos |
Previous Message | Michael Paquier | 2023-05-15 07:33:27 | Re: createuser --memeber and PG 16 |