Fujii Masao wrote:
> On Tue, Aug 26, 2014 at 3:27 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
> > Fujii Masao wrote:
> >> On Thu, Aug 21, 2014 at 4:20 PM, Michael Paquier
> >> <michael.paquier@gmail.com> wrote:
> >
> >> > Looking at the patch, the parameter "fillfactor" in the category
> >> > RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is
> >> > not updated with the new field. It is only a one-line change.
> >> > @@ -97,7 +97,7 @@ static relopt_int intRelOpts[] =
> >> > "Packs table pages only to this percentage",
> >> > RELOPT_KIND_HEAP
> >> > },
> >> > - HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
> >> > + HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0
> >> > },
> >>
> >> Oh, good catch. I wonder why I did such a mistake...
> >
> > Uninitialized elements at end of struct are filled with zeroes.
>
> Yeah, that's the reason why I could not notice the problem at compile time.
Right -- it's not something the compiler would warn you about.
> > We do
> > have other examples of this -- for instance, config_generic in the guc.c
> > tables are almost always only 5 members long even though the struct is
> > quite a bit longer than that. Most entries do not even have "flags" set.
>
> So you imply that the trailing zero (which the patch adds as flag)
> in the reloption struct should be dropped?
Not necessarily, because it's harmless. It's there for purely
aesthetical reasons, so it's your choice whether to add it or not.
Having it there is slightly easier on somebody reading the code,
perhaps.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services