Re: pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs

At Thu, 15 Mar 2018 15:09:54 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20180315060954.GE617@paquier.xyz>
> On Thu, Mar 15, 2018 at 02:03:15PM +0900, Kyotaro HORIGUCHI wrote:
> > At Thu, 15 Mar 2018 00:33:25 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <22193.1521088405@sss.pgh.pa.us>
> >> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> >>> Doesn't it make sense if we provide a buildtime-script that
> >>> collects the function names and builds a .h file containing a
> >>> function using the list?
> >> 
> >> Surely this is a fundamentally misguided approach.  How could it
> >> handle extension GUCs?
> > 
> > I understand it is "out of scope" of this thread (for now). The
> > starting issue here is "the static list of list-guc's are stale"
> > so what a static list cannot cope with is still cannot be coped
> > with by this.
> 
> I like the mention of the idea, now it is true that that it would be a
> half-baked work without parameters from plpgsql, and a way to correctly
> track parameters marking a custom GUC as GUC_INPUT_LIST in all C files.
> 
> > Or, we could cope with this issue if the list-ness of used GUCs
> > is stored permanently in somewhere. Maybe pg_proc.proconfigislist
> > or something...
> 
> That does not help for PL's GUCs as well.  Those may not be loaded when
> pg_get_functiondef gets called.

So, we should reject to define function in the case. We don't
accept the GUC element if it is just a placeholder.

The attached is a rush work of my idea. Diff for pg_proc.h is too
large so it is separated and gziped.

It adds a column named "proconfigislist" of array(bool) in
pg_proc. When defined function has set clauses, it generates a
proconfig-is-list-or-not array and set. It ends with error if
required module is not loaded yet. Perhaps
GetConfigOptionFlags(,false) shouldn't return 0 if no option
element is found but I don't amend it for now.

Finally, it works as the follows. I think this leands to a kind
of desired behavior.

======
postgres=# CREATE FUNCTION func_with_set_params() RETURNS integer
    AS 'select 1;'
    LANGUAGE SQL
    set plpgsql.extra_errors to 'shadowed_variables'
    set work_mem to '48MB'
    set plpgsql.extra_warnings to 'shadowed_variables';
ERROR:  the module for variable "plpgsql.extra_errors" is not loaded yet
DETAIL:  The module must be loaded before referring this variable.
postgres=# load 'plpgsql';
postgres=# CREATE FUNCTION func_with_set_params() RETURNS integer
    AS 'select 1;'
    LANGUAGE SQL
    set plpgsql.extra_errors to 'shadowed_variables'
    set work_mem to '48MB'
    set plpgsql.extra_warnings to 'shadowed_variables';
postgres=# select proname, proconfig, proconfigislist from pg_proc where proconfig is not null;
-[ RECORD 1 ]---+--------------------------------------------------------------------------------------------------
proname         | func_with_set_params
proconfig       | {plpgsql.extra_errors=shadowed_variables,work_mem=48MB,plpgsql.extra_warnings=shadowed_variables}
proconfigislist | {t,f,t}
=======

pg_get_functiondef() can work correctly with this even if
required modules are not loaded.

But, I suppose it is a bit too big.

> At the end, we are spending a lot of resources on that.  So in order to
> close this thread, I would suggest to just complete the list of
> hardcoded parameters on backend and frontend, and add as well a comment
> including "GUC_INPUT_LIST" so as it can be found by grepping the code.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center