Обсуждение: Refactoring GUC unit conversions
In the "redesign checkpoint_segments" patch, Robert suggested keeping the settings' base unit as "number of segments", but allow conversions from MB, GB etc. I started looking into that and found that adding a new unit to guc.c is quite messy. The conversions are done with complicated if-switch-case constructs. Attached is a patch to refactor that, making the conversions table-driven. This doesn't change functionality, it just makes the code nicer. Any objections? - Heikki
Вложения
On 2/13/15 7:26 AM, Heikki Linnakangas wrote: > In the "redesign checkpoint_segments" patch, Robert suggested keeping > the settings' base unit as "number of segments", but allow conversions > from MB, GB etc. I started looking into that and found that adding a new > unit to guc.c is quite messy. The conversions are done with complicated > if-switch-case constructs. > > Attached is a patch to refactor that, making the conversions > table-driven. This doesn't change functionality, it just makes the code > nicer. Looks good, but shouldn't there be a check for a unit that's neither memory or time? -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
On 02/13/2015 07:34 PM, Jim Nasby wrote: > On 2/13/15 7:26 AM, Heikki Linnakangas wrote: >> In the "redesign checkpoint_segments" patch, Robert suggested keeping >> the settings' base unit as "number of segments", but allow conversions >> from MB, GB etc. I started looking into that and found that adding a new >> unit to guc.c is quite messy. The conversions are done with complicated >> if-switch-case constructs. >> >> Attached is a patch to refactor that, making the conversions >> table-driven. This doesn't change functionality, it just makes the code >> nicer. > > Looks good, but shouldn't there be a check for a unit that's neither > memory or time? Can you elaborate? We currently only support units for memory and time settings. - Heikki
On 2/13/15 11:44 AM, Heikki Linnakangas wrote: > On 02/13/2015 07:34 PM, Jim Nasby wrote: >> On 2/13/15 7:26 AM, Heikki Linnakangas wrote: >>> In the "redesign checkpoint_segments" patch, Robert suggested keeping >>> the settings' base unit as "number of segments", but allow conversions >>> from MB, GB etc. I started looking into that and found that adding a new >>> unit to guc.c is quite messy. The conversions are done with complicated >>> if-switch-case constructs. >>> >>> Attached is a patch to refactor that, making the conversions >>> table-driven. This doesn't change functionality, it just makes the code >>> nicer. >> >> Looks good, but shouldn't there be a check for a unit that's neither >> memory or time? > > Can you elaborate? We currently only support units for memory and time > settings. I'm thinking an Assert in case someone screws up the function call. But perhaps I'm just being paranoid. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
On Fri, Feb 13, 2015 at 10:26 PM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > In the "redesign checkpoint_segments" patch, Robert suggested keeping the > settings' base unit as "number of segments", but allow conversions from MB, > GB etc. I started looking into that and found that adding a new unit to > guc.c is quite messy. The conversions are done with complicated > if-switch-case constructs. > > Attached is a patch to refactor that, making the conversions table-driven. > This doesn't change functionality, it just makes the code nicer. Isn't it good idea to allow even wal_keep_segments to converse from MB, GB etc? Regards, -- Fujii Masao