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 |
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) |