On Sat, Jun 27, 2015 at 7:19 PM, Tom Lane <
tgl@sss.pgh.pa.us> wrote:
>
> Amit Kapila <
amit.kapila16@gmail.com> writes:
> > On Fri, Jun 26, 2015 at 9:47 PM, Tom Lane <
tgl@sss.pgh.pa.us> wrote:
> >> What we evidently need to do is fix things so that the pg_file_settings
> >> data gets captured before we suppress duplicates.
> >>
> >> The simplest change would be to move the whole thing to around line 208 in
> >> guc-file.l, just after the stanza that loads PG_AUTOCONF_FILENAME. Or you
> >> could argue that the approach is broken altogether, and that we should
> >> capture the data while we read the files, so that you have some useful
> >> data in the view even if ParseConfigFile later decides there's a syntax
> >> error. I'm actually thinking maybe we should flush that data-capturing
> >> logic altogether in favor of just not deleting the ConfigVariable list
> >> data structure, and generating the view directly from that data structure.
>
> > Idea for generating view form ConfigVariable list sounds good, but how
> > will it preserve the duplicate entries in the list assuming either we need
> > to revert the original fix (e3da0d4d1) or doing the same in loop where
> > we set GUC_IS_IN_FILE?
>
> I'm thinking of adding an "ignore" boolean flag to ConfigVariable, which
> the GUC_IS_IN_FILE loop would set in ConfigVariables that are superseded
> by later list entries. Then the GUC-application loop would just skip
> those entries. This would be good because the flag could be displayed
> somehow in the pg_file_settings view, whereas right now you have to
> manually check for duplicates.
>
Sounds good way to deal with this problem.
> > Keeping removal of duplicate items in ParseConfigFp() has the advantage
> > that it will work for all other places from where ParseConfigFp() is called,
> > though I am not sure if today that is required.
>
> I don't think it is; if it were, we'd have had other complaints about
> that, considering that 9.4.0 is the *only* release we've ever shipped
> that suppressed duplicates right inside ParseConfigFp(). I would in
> fact turn that argument on its head, and state that Fujii-san's patch
> was probably ill-conceived because it implicitly assumes that duplicate
> suppression is okay for every caller of ParseConfigFp.
I have implemented that patch, so if you see any problem's with that
approach, Fujji-san is not right person to blame.