On Sat, Jun 27, 2015 at 7:19 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Amit Kapila <amit.kapil...@gmail.com> writes: > > On Fri, Jun 26, 2015 at 9:47 PM, Tom Lane <t...@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. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
