Re: pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs

2018-03-16 9:34 GMT+01:00 Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp
>:

> At Fri, 16 Mar 2018 09:16:54 +0100, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote in <CAFj8pRC6+_8c6PLHbDzCSKEDpkEhcVqyFmJn9_
> zYUjAwdnUboQ(at)mail(dot)gmail(dot)com>
> > 2018-03-16 5:46 GMT+01:00 Michael Paquier <michael(at)paquier(dot)xyz>:
> >
> > > On Fri, Mar 16, 2018 at 10:25:35AM +0900, Michael Paquier wrote:
> > > >> But, I suppose it is a bit too big.
> > > >
> > > > That's of course not backpatchable.
>
> I meant that it is just too big for the objective regardless of
> back-patching:p
>
> So, automatic generation is not acceptable, dynamic checking
> (including that at the function creation) would be
> unacceptable. So I agree that what we can do now is just maintian
> the list of pg_strcasecmp.
>
> > > So in this jungle attached is my counter-proposal. As the same code
> > > pattern is repeated in three places, we could as well refactor the
> whole
> > > into a common routine, say in src/common/guc_options.c or similar.
> > > Perhaps just on HEAD and not back-branches as this is always annoying
> > > for packagers on Windows using custom scripts. Per the lack of
> > > complains only doing something on HEAD, with only a subset of the
> > > parameters which make sense for CREATE FUNCTION is what makes the most
> > > sense in my opinion.
> > >
> >
> > Last patch is good enough solution, I tested it and it is working
> >
> > Little bit strange is support of GUC, that cannot be atteched to any fx.
> > Probably only PGC_USERSET, maybe PGC_SUSET has sense there
>
> That's true, but doesn't the additional rule make it more
> bothersome to maintain the list?
>

Hard to say. I have not opinion. But just when I see in this context
variables like listen_address, shared_preload_librarries, then it looks
messy.

>
> > + if (pg_strcasecmp(configitem, "DateStyle") == 0 ||
> > + pg_strcasecmp(configitem, "listen_addresses") == 0
> ||
> > + pg_strcasecmp(configitem, "local_preload_libraries")
> > == 0 ||
> > + pg_strcasecmp(configitem, "log_destination") == 0 ||
> > + pg_strcasecmp(configitem, "plpgsql.extra_errors")
> == 0
> > ||
> > + pg_strcasecmp(configitem, "plpgsql.extra_warnings")
> ==
> > 0 ||
> > + pg_strcasecmp(configitem, "search_path") == 0 ||
> > + pg_strcasecmp(configitem,
> "session_preload_libraries")
> > == 0 ||
> > + pg_strcasecmp(configitem,
> "shared_preload_libraries")
> > == 0 ||
> > + pg_strcasecmp(configitem,
> "synchronous_standby_names")
> > == 0 ||
> > + pg_strcasecmp(configitem, "temp_tablespaces") == 0
> ||
> > + pg_strcasecmp(configitem,
> "wal_consistency_checking")
> > == 0)
> >
> >
> > another idea, how to solve this issue for extensions. The information
> about
> > format of extensions GUC can be stored in new column pg_extension - maybe
> > just a array of list of LIST GUC.
> >
> > CREATE EXTENSION plpgsql ...
> > GUC_PREFIX 'plpgsql'
> > LIST_GUC ('extra_errors','extra_warnings')
> > ...
> >
> > Regards
> >
> > Pavel
> >
> >
> >
> > >
> > > As mentioned in this bug, the handling of empty values gets kind of
> > > tricky as in this case proconfig stores a set of quotes and not an
> > > actual value:
> > > https://www.postgresql.org/message-id/152049236165.23137.
> > > 5241258464332317087(at)wrigleys(dot)postgresql(dot)org
> > > --
> > > Michael
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>
>