Re: pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: michael(at)paquier(dot)xyz
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, pavel(dot)stehule(at)gmail(dot)com, a(dot)zakirov(at)postgrespro(dot)ru, michael(dot)paquier(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs
Date: 2018-03-15 09:48:36
Message-ID: 20180315.184836.185034889.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Thu, 15 Mar 2018 15:09:54 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20180315060954(dot)GE617(at)paquier(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote in <22193(dot)1521088405(at)sss(dot)pgh(dot)pa(dot)us>
> >> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)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

Attachment Content-Type Size
0001-Store-whether-elements-of-proconfig-are-list-or-not-.patch text/x-patch 10.0 KB
0002-pg_proc.h-part.patch.gz application/octet-stream 77.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message leap 2018-03-15 10:08:30 Re:Re: [submit code] I develop a tool for pgsql, how can I submit it
Previous Message Pranav Negi 2018-03-15 09:35:21 GSOC :Thrift datatype support (2018)