Re: Semantics of pg_file_settings view

Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> On Fri, Jun 26, 2015 at 9:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

> 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. It's not hard
to imagine use-cases that that would break, even if we have none today.

regards, tom lane