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.

> 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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to