RE: [PoC] pg_upgrade: allow to upgrade publisher node - Mailing list pgsql-hackers

From Hayato Kuroda (Fujitsu)
Subject RE: [PoC] pg_upgrade: allow to upgrade publisher node
Date
Msg-id TYAPR01MB58664CB26AAA51EACA1C9B33F5799@TYAPR01MB5866.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: [PoC] pg_upgrade: allow to upgrade publisher node  (Peter Smith <smithpb2250@gmail.com>)
Responses Re: [PoC] pg_upgrade: allow to upgrade publisher node
RE: [PoC] pg_upgrade: allow to upgrade publisher node
List pgsql-hackers
Dear Peter,

Thanks for reviewing! PSA new version patchset.

> 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];

Right, fixed.

> 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_VER
BOSE, "Database: %s", pDbInfo->db_name);
> 

Oh, I missed it. Fixed. I grepped patches and could not find other lines
which should be double-quoted.

In addition, I ran pgindent again for 0001.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment

pgsql-hackers by date:

Previous
From: "Drouvot, Bertrand"
Date:
Subject: Re: Introduce WAIT_EVENT_EXTENSION and WAIT_EVENT_BUFFER_PIN
Next
From: Michael Paquier
Date:
Subject: Re: Introduce WAIT_EVENT_EXTENSION and WAIT_EVENT_BUFFER_PIN