pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs
Hi all,
While reviewing another patch related to the use of pg_strcasecmp in the
backend, I have noticed this bit in ruleutils.c:
/*
* Some GUC variable names are 'LIST' type and hence must not
* be quoted.
*/
if (pg_strcasecmp(configitem, "DateStyle") == 0
|| pg_strcasecmp(configitem, "search_path") == 0)
appendStringInfoString(&buf, pos);
else
simple_quote_literal(&buf, pos);
However this fails to consider all GUCs marked as GUC_LIST_INPUT, like
the recent wal_consistency_checking. guc.c already holds a find_option()
which can be used to retrieve the flags of a parameter. What about using
that and filtering by GUC_LIST_INPUT? This requires exposing the
function, and I am not sure what people think about that.
Thoughts?
--
Michael
Michael Paquier <michael.paquier@gmail.com> writes:
While reviewing another patch related to the use of pg_strcasecmp in the
backend, I have noticed this bit in ruleutils.c:
/*
* Some GUC variable names are 'LIST' type and hence must not
* be quoted.
*/
if (pg_strcasecmp(configitem, "DateStyle") == 0
|| pg_strcasecmp(configitem, "search_path") == 0)
appendStringInfoString(&buf, pos);
else
simple_quote_literal(&buf, pos);
However this fails to consider all GUCs marked as GUC_LIST_INPUT, like
the recent wal_consistency_checking.
Mmm, yeah, probably time to find a more maintainable solution to that.
guc.c already holds a find_option()
which can be used to retrieve the flags of a parameter. What about using
that and filtering by GUC_LIST_INPUT? This requires exposing the
function, and I am not sure what people think about that.
Don't like exposing find_option directly, but I think it would make
sense to provide an API along the lines of
int GetConfigOptionFlags(const char *name, bool missing_ok).
regards, tom lane
On Thu, Jan 11, 2018 at 10:42:38AM -0500, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
guc.c already holds a find_option()
which can be used to retrieve the flags of a parameter. What about using
that and filtering by GUC_LIST_INPUT? This requires exposing the
function, and I am not sure what people think about that.Don't like exposing find_option directly, but I think it would make
sense to provide an API along the lines of
int GetConfigOptionFlags(const char *name, bool missing_ok).
OK, I can live with that. What do you think about the attached? I'll be
happy to produce patches for back-branches as necessary. When an option
is not found, I have made the function return 0 as value for the flags,
which is consistent with flatten_set_variable_args(). To make things
behave more consistently with GUC_LIST_QUOTE GUCs, it seems to me that
those should not be quoted as well (ALTER SYSTEM shares the same
compatibility). And attached is a patch.
While reviewing more the code, I have noticed the same code pattern in
pg_dump.c and pg_dumpall.c. In the patch attached, I have corrected
things so as all parameters marked as GUC_LIST_QUOTE are correctly not
quoted because we have no generic solution to know from frontends what's
a GUC type (it would make sense to me to expose a text[] with this
information in pg_settings). However, let's be honest, it does not make
sense to update all those parameters because who is really going to use
them for functions! Two things that make sense to me are just
wal_consistency_checking and synchronous_standby_names for developers
which could use it to tune WAL generated within a set of SQL or plpgsql
functions. As it is easier to delete than add code here, I have added
all of them to ease the production of a committable version. For
correctness, still having them may make sense.
--
Michael
Attachments:
guc-list-track-v1.patchtext/plain; charset=us-asciiDownload
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 9cdbb06add..7914f4dd88 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -61,6 +61,7 @@
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/hsearch.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
@@ -2561,6 +2562,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
{
char *configitem = TextDatumGetCString(d);
char *pos;
+ int flags;
pos = strchr(configitem, '=');
if (pos == NULL)
@@ -2571,11 +2573,11 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
quote_identifier(configitem));
/*
- * Some GUC variable names are 'LIST' type and hence must not
- * be quoted.
+ * Some GUC variable names are of type GUC_LIST_INPUT
+ * and hence must not be quoted.
*/
- if (pg_strcasecmp(configitem, "DateStyle") == 0
- || pg_strcasecmp(configitem, "search_path") == 0)
+ flags = GetConfigOptionFlags(configitem, false);
+ if ((flags & GUC_LIST_INPUT) != 0)
appendStringInfoString(&buf, pos);
else
simple_quote_literal(&buf, pos);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 72f6be329e..5a39c96242 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8099,6 +8099,30 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok)
return _ShowOption(record, true);
}
+/*
+ * Return "flags" for a given GUC variable. If missing_ok is true, ignore
+ * any errors and return 0 to the caller instead.
+ */
+int
+GetConfigOptionFlags(const char *name, bool missing_ok)
+{
+ struct config_generic *record;
+
+ record = find_option(name, false, ERROR);
+
+ if (record == NULL)
+ {
+ if (missing_ok)
+ return 0;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ }
+
+ return record->flags;
+}
+
/*
* Return GUC variable value by variable number; optionally return canonical
* form of name. Return value is palloc'd.
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 27628a397c..f9b9de8894 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11777,11 +11777,20 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
appendPQExpBuffer(q, "\n SET %s TO ", fmtId(configitem));
/*
- * Some GUC variable names are 'LIST' type and hence must not be
- * quoted.
+ * Some GUC variable names are of type GUC_LIST_INPUT and hence
+ * must not be quoted.
+ * XXX: Keep this list in sync with what is defined in guc.c!
*/
- if (pg_strcasecmp(configitem, "DateStyle") == 0
- || pg_strcasecmp(configitem, "search_path") == 0)
+ 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, "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)
appendPQExpBufferStr(q, pos);
else
appendStringLiteralAH(q, pos, fout);
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 3dd2c3871e..2fd43b039e 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1728,10 +1728,20 @@ makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
appendPQExpBuffer(buf, "SET %s TO ", fmtId(mine));
/*
- * Some GUC variable names are 'LIST' type and hence must not be quoted.
+ * Some GUC variable names are of type GUC_LIST_INPUT and hence
+ * must not be quoted.
+ * XXX: Keep this list in sync with what is defined in guc.c!
*/
- if (pg_strcasecmp(mine, "DateStyle") == 0
- || pg_strcasecmp(mine, "search_path") == 0)
+ if (pg_strcasecmp(mine, "DateStyle") == 0 ||
+ pg_strcasecmp(mine, "listen_addresses") == 0 ||
+ pg_strcasecmp(mine, "local_preload_libraries") == 0 ||
+ pg_strcasecmp(mine, "log_destination") == 0 ||
+ pg_strcasecmp(mine, "search_path") == 0 ||
+ pg_strcasecmp(mine, "session_preload_libraries") == 0 ||
+ pg_strcasecmp(mine, "shared_preload_libraries") == 0 ||
+ pg_strcasecmp(mine, "synchronous_standby_names") == 0 ||
+ pg_strcasecmp(mine, "temp_tablespaces") == 0 ||
+ pg_strcasecmp(mine, "wal_consistency_checking") == 0)
appendPQExpBufferStr(buf, pos + 1);
else
appendStringLiteralConn(buf, pos + 1, conn);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 77daa5a539..b02e65cf59 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -368,6 +368,7 @@ extern int set_config_option(const char *name, const char *value,
extern void AlterSystemSetConfigFile(AlterSystemStmt *setstmt);
extern char *GetConfigOptionByName(const char *name, const char **varname,
bool missing_ok);
+extern int GetConfigOptionFlags(const char *name, bool missing_ok);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index f1c1b44d6f..c7ebb29b65 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3226,6 +3226,30 @@ SELECT pg_get_partkeydef(0);
(1 row)
+-- test for pg_get_functiondef with list parameters which should not be
+-- quoted.
+CREATE FUNCTION func_with_set_params() RETURNS integer
+ AS 'select 1;'
+ LANGUAGE SQL
+ SET search_path TO 'pg_catalog'
+ SET wal_consistency_checking TO heap, heap2
+ SET session_preload_libraries TO foo, bar
+ IMMUTABLE STRICT;
+SELECT pg_get_functiondef('func_with_set_params'::regproc);
+ pg_get_functiondef
+----------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.func_with_set_params()+
+ RETURNS integer +
+ LANGUAGE sql +
+ IMMUTABLE STRICT +
+ SET search_path TO pg_catalog +
+ SET wal_consistency_checking TO heap, heap2 +
+ SET session_preload_libraries TO foo, bar +
+ AS $function$select 1;$function$ +
+
+(1 row)
+
+DROP FUNCTION func_with_set_params();
-- test rename for a rule defined on a partitioned table
CREATE TABLE parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1);
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 0ded0f01d2..40a6914e69 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1170,6 +1170,18 @@ SELECT pg_get_function_arg_default(0, 0);
SELECT pg_get_function_arg_default('pg_class'::regclass, 0);
SELECT pg_get_partkeydef(0);
+-- test for pg_get_functiondef with list parameters which should not be
+-- quoted.
+CREATE FUNCTION func_with_set_params() RETURNS integer
+ AS 'select 1;'
+ LANGUAGE SQL
+ SET search_path TO 'pg_catalog'
+ SET wal_consistency_checking TO heap, heap2
+ SET session_preload_libraries TO foo, bar
+ IMMUTABLE STRICT;
+SELECT pg_get_functiondef('func_with_set_params'::regproc);
+DROP FUNCTION func_with_set_params();
+
-- test rename for a rule defined on a partitioned table
CREATE TABLE parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1);
Hello,
On Fri, Jan 12, 2018 at 10:24:40AM +0900, Michael Paquier wrote:
OK, I can live with that. What do you think about the attached? I'll be
happy to produce patches for back-branches as necessary. When an option
is not found, I have made the function return 0 as value for the flags,
which is consistent with flatten_set_variable_args(). To make things
behave more consistently with GUC_LIST_QUOTE GUCs, it seems to me that
those should not be quoted as well (ALTER SYSTEM shares the same
compatibility). And attached is a patch.
Just 2 cents from me. It seems that there is a problem with extensions
GUCs. For example:
=# CREATE FUNCTION func_with_set_params() RETURNS integer
AS 'select 1;'
LANGUAGE SQL
set plpgsql.extra_errors to 'shadowed_variables';
=# SELECT pg_get_functiondef('func_with_set_params'::regproc);
pg_get_functiondef
----------------------------------------------------------
CREATE OR REPLACE FUNCTION public.func_with_set_params()+
RETURNS integer +
LANGUAGE sql +
SET "plpgsql.extra_errors" TO 'shadowed_variables' +
AS $function$select 1;$function$ +
It is good while plpgsql is loaded. But when we exit the session and try
it again in another:
=# SELECT pg_get_functiondef('func_with_set_params'::regproc);
ERROR: unrecognized configuration parameter "plpgsql.extra_errors"
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
On Tue, Feb 20, 2018 at 06:46:57PM +0300, Arthur Zakirov wrote:
Just 2 cents from me. It seems that there is a problem with extensions
GUCs.[...]
=# SELECT pg_get_functiondef('func_with_set_params'::regproc);
ERROR: unrecognized configuration parameter "plpgsql.extra_errors"
You have an excellent point here, and I did not consider it.
For pg_dump and pg_dumpall, something like what I described in
/messages/by-id/20180112012440.GA2222@paquier.xyz
to extend pg_settings so as GUC types are visible via SQL could make
sense, now it is really a lot for just being able to quote parameters
correctly. On top of that, what I suggested previously would not work
reliably except if we have pg_get_functiondef load the library
associated to a given parameter. Even in this case there could
perfectly be a custom parameter from another plugin and not a parameter
associated to the function language itself.
It seems to me that this brings us back to a best-effort for the backend
and the frontend by maintaining a list of hardcoded parameter names,
which could be refined a maximum by considering lists for in-core
extensions and perhaps the most famous extensions around :(
Thoughts?
--
Michael
On Wed, Feb 21, 2018 at 04:35:50PM +0900, Michael Paquier wrote:
You have an excellent point here, and I did not consider it.
For pg_dump and pg_dumpall, something like what I described in
/messages/by-id/20180112012440.GA2222@paquier.xyz
to extend pg_settings so as GUC types are visible via SQL could make
sense, now it is really a lot for just being able to quote parameters
correctly. On top of that, what I suggested previously would not work
reliably except if we have pg_get_functiondef load the library
associated to a given parameter. Even in this case there could
perfectly be a custom parameter from another plugin and not a parameter
associated to the function language itself.
In my opinion GetConfigOptionFlags() can be used for core and plpgsql GUCs.
A variable should not be quoted if it has GUC_LIST_INPUT flag or it is
plpgsql.extra_warnings or plpgsql.extra_errors.
I'm not sure that it is good to raise an error when the variable isn't
found, because without the patch the error isn't raised. But maybe better
to raise it to notify a user that there is a wrong variable. In this case we
may not raise the error if variable name contains
GUC_QUALIFIER_SEPARATOR.
It seems to me that this brings us back to a best-effort for the backend
and the frontend by maintaining a list of hardcoded parameter names,
which could be refined a maximum by considering lists for in-core
extensions and perhaps the most famous extensions around :(
It seems for frontend maintaining a hardcoded list is the only way.
Agree that extending pg_settings for that could be too much.
I've searched extensions in GitHub with GUC_LIST_INPUT variables. There
are couple of them:
- https://github.com/ohmu/pgmemcache
- https://github.com/marconeperes/pgBRTypes
And some not very fresh:
- https://github.com/witblitz/pldotnet
- https://github.com/ohmu/pgloggingfilter
- https://github.com/wangyuehong/pggearman
- https://github.com/siavashg/pgredis
--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
2018-02-21 8:35 GMT+01:00 Michael Paquier <michael@paquier.xyz>:
On Tue, Feb 20, 2018 at 06:46:57PM +0300, Arthur Zakirov wrote:
Just 2 cents from me. It seems that there is a problem with extensions
GUCs.[...]
=# SELECT pg_get_functiondef('func_with_set_params'::regproc);
ERROR: unrecognized configuration parameter "plpgsql.extra_errors"You have an excellent point here, and I did not consider it.
For pg_dump and pg_dumpall, something like what I described in
/messages/by-id/20180112012440.GA2222@paquier.xyz
to extend pg_settings so as GUC types are visible via SQL could make
sense, now it is really a lot for just being able to quote parameters
correctly. On top of that, what I suggested previously would not work
reliably except if we have pg_get_functiondef load the library
associated to a given parameter. Even in this case there could
perfectly be a custom parameter from another plugin and not a parameter
associated to the function language itself.It seems to me that this brings us back to a best-effort for the backend
and the frontend by maintaining a list of hardcoded parameter names,
which could be refined a maximum by considering lists for in-core
extensions and perhaps the most famous extensions around :(
I afraid so there is not good solution. Is possible to store options in
original form? When the function will be displayed, then the original value
will be displayed. The current patch (with known extensions) can be used as
bug fix and back patched. In new version we store original value with
quotes.
Regards
Pavel
Show quoted text
Thoughts?
--
Michael
On Mon, Mar 05, 2018 at 09:25:09PM +0100, Pavel Stehule wrote:
I afraid so there is not good solution. Is possible to store options in
original form? When the function will be displayed, then the original value
will be displayed. The current patch (with known extensions) can be used as
bug fix and back patched. In new version we store original value with
quotes.
You mean storing the values in pg_proc.proconfig at the creation time of
the function? That would basically work, except for the case of a
function using a parameter which is not from the same PL. The function
creation would fail because it cannot find the parameter it is looking
for as we need to look at loaded parameters to know it uses a list or
not :(
--
Michael
2018-03-06 2:32 GMT+01:00 Michael Paquier <michael@paquier.xyz>:
On Mon, Mar 05, 2018 at 09:25:09PM +0100, Pavel Stehule wrote:
I afraid so there is not good solution. Is possible to store options in
original form? When the function will be displayed, then the originalvalue
will be displayed. The current patch (with known extensions) can be used
as
bug fix and back patched. In new version we store original value with
quotes.You mean storing the values in pg_proc.proconfig at the creation time of
the function? That would basically work, except for the case of a
function using a parameter which is not from the same PL. The function
creation would fail because it cannot find the parameter it is looking
for as we need to look at loaded parameters to know it uses a list or
not :(
yes. It can fails on execution time, but it is something like runtime error.
just dump should to produce same form like was input. So if on input we got
quotes, then we should to use quotes on output. Without querying somewhere.
The possible quotes can be removed in function compile time.
Pavel
Show quoted text
--
Michael
Hello,
At Tue, 6 Mar 2018 07:14:00 +0100, Pavel Stehule <pavel.stehule@gmail.com> wrote in <CAFj8pRB7u-D1NA8a22dytwicKv4RWrYqKCF=yiL5kKMKbssPSw@mail.gmail.com>
2018-03-06 2:32 GMT+01:00 Michael Paquier <michael@paquier.xyz>:
On Mon, Mar 05, 2018 at 09:25:09PM +0100, Pavel Stehule wrote:
I afraid so there is not good solution. Is possible to store options in
original form? When the function will be displayed, then the originalvalue
will be displayed. The current patch (with known extensions) can be used
as
bug fix and back patched. In new version we store original value with
quotes.You mean storing the values in pg_proc.proconfig at the creation time of
the function? That would basically work, except for the case of a
function using a parameter which is not from the same PL. The function
creation would fail because it cannot find the parameter it is looking
for as we need to look at loaded parameters to know it uses a list or
not :(yes. It can fails on execution time, but it is something like runtime error.
just dump should to produce same form like was input. So if on input we got
quotes, then we should to use quotes on output. Without querying somewhere.The possible quotes can be removed in function compile time.
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?
The attached perl script is a rush work of such script, which
works at the top of the source tree. It just prints the function
definition, does not generate a .h file.
I haven't confirmed anything about it but I had the following
output from the current master.
inline bool
IsConfigOptionIsAList(const char *name){
if (pg_strcasecmp(name, "DateStyle") == 0
|| pg_strcasecmp(name, "temp_tablespaces") == 0
|| pg_strcasecmp(name, "session_preload_libraries") == 0
|| pg_strcasecmp(name, "shared_preload_libraries") == 0
|| pg_strcasecmp(name, "local_preload_libraries") == 0
|| pg_strcasecmp(name, "search_path") == 0
|| pg_strcasecmp(name, "log_destination") == 0
|| pg_strcasecmp(name, "listen_addresses") == 0
|| pg_strcasecmp(name, "synchronous_standby_names") == 0
|| pg_strcasecmp(name, "wal_consistency_checking") == 0)
return true;
return false;
}
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
On Wed, Mar 14, 2018 at 05:30:59PM +0900, Kyotaro HORIGUCHI wrote:
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?The attached perl script is a rush work of such script, which
works at the top of the source tree. It just prints the function
definition, does not generate a .h file.I haven't confirmed anything about it but I had the following
output from the current master.
I quite like that idea actually. Please note that pl_handler.c declares
two more of them, so we may want a smarter parsing which takes into
account declarations of DefineCustom*Variable.
--
Michael
Hello,
At Wed, 14 Mar 2018 17:46:21 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20180314084621.GA617@paquier.xyz>
On Wed, Mar 14, 2018 at 05:30:59PM +0900, Kyotaro HORIGUCHI wrote:
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?The attached perl script is a rush work of such script, which
works at the top of the source tree. It just prints the function
definition, does not generate a .h file.I haven't confirmed anything about it but I had the following
output from the current master.I quite like that idea actually. Please note that pl_handler.c declares
two more of them, so we may want a smarter parsing which takes into
account declarations of DefineCustom*Variable.
Only DefineCustomStringVariable can accept a list argument even
if flagged GUC_LIST_INPUT. (I'm not sure this should be
explicitly rejected.) I'm not sure this is smart enough but it
works. It would fail if description for variables contain the
same character sequence to the lexical structure around but it is
rare to happen.
The attached patch is the result. It adds a script to generate
include/common/check_listvars.h. It won't be updated even if
related files are modifed (since we cannot know it before
scanning the whole source.) but "make clean" removes it. Regtests
is a copy of Michael's v1 patch.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
guc-list-track-v2.patchtext/x-patch; charset=us-asciiDownload
From 17cadb17279c802c79e4afa0d7fcd7f63ed38d03 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 15 Mar 2018 13:12:17 +0900
Subject: [PATCH] Autogenerating function to detect list-formatted GUC
variables
Some features require to know whether a GUC variables is list or not
but the set of such functions can be modified through development. We
tried to check it on-the-fly but found that that makes things more
complex. Instead, this generates the list of such variables by
scanning the whole source tree, instead of manually maintaining it.
---
src/backend/Makefile | 4 +-
src/backend/utils/adt/ruleutils.c | 4 +-
src/bin/pg_dump/Makefile | 4 +-
src/bin/pg_dump/dumputils.c | 5 +-
src/bin/pg_dump/pg_dump.c | 4 +-
src/include/Makefile | 7 +-
src/include/gen_listvars.pl | 145 ++++++++++++++++++++++++++++++++++++
src/test/regress/expected/rules.out | 24 ++++++
src/test/regress/sql/rules.sql | 12 +++
9 files changed, 197 insertions(+), 12 deletions(-)
create mode 100755 src/include/gen_listvars.pl
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 4a28267339..10656c53e6 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -169,7 +169,7 @@ submake-schemapg:
.PHONY: generated-headers
-generated-headers: $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/fmgrprotos.h $(top_builddir)/src/include/utils/probes.h
+generated-headers: $(top_builddir)/src/include/parser/gram.h $(top_builddir)/src/include/catalog/schemapg.h $(top_builddir)/src/include/storage/lwlocknames.h $(top_builddir)/src/include/utils/errcodes.h $(top_builddir)/src/include/utils/fmgroids.h $(top_builddir)/src/include/utils/fmgrprotos.h $(top_builddir)/src/include/utils/probes.h $(top_builddir)/src/include/common/check_listvars.h
$(top_builddir)/src/include/parser/gram.h: parser/gram.h
prereqdir=`cd '$(dir $<)' >/dev/null && pwd` && \
@@ -205,6 +205,8 @@ $(top_builddir)/src/include/utils/probes.h: utils/probes.h
cd '$(dir $@)' && rm -f $(notdir $@) && \
$(LN_S) "../../../$(subdir)/utils/probes.h" .
+$(top_builddir)/src/include/common/check_listvars.h:
+ $(MAKE) -C $(dir $@).. common/check_listvars.h
utils/probes.o: utils/probes.d $(SUBDIROBJS)
$(DTRACE) $(DTRACEFLAGS) -C -G -s $(call expand_subsys,$^) -o $@
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b58ee3c387..d2ad034e06 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -41,6 +41,7 @@
#include "catalog/pg_type.h"
#include "commands/defrem.h"
#include "commands/tablespace.h"
+#include "common/check_listvars.h"
#include "common/keywords.h"
#include "executor/spi.h"
#include "funcapi.h"
@@ -2599,8 +2600,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
* Some GUC variable names are 'LIST' type and hence must not
* be quoted.
*/
- if (pg_strcasecmp(configitem, "DateStyle") == 0
- || pg_strcasecmp(configitem, "search_path") == 0)
+ if (IsConfigOptionAList(configitem))
appendStringInfoString(&buf, pos);
else
simple_quote_literal(&buf, pos);
diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile
index e3bfc95f16..d79ac49a83 100644
--- a/src/bin/pg_dump/Makefile
+++ b/src/bin/pg_dump/Makefile
@@ -25,13 +25,13 @@ OBJS= pg_backup_archiver.o pg_backup_db.o pg_backup_custom.o \
all: pg_dump pg_restore pg_dumpall
-pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
+pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils submake-generated-headers
$(CC) $(CFLAGS) pg_dump.o common.o pg_dump_sort.o $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
pg_restore: pg_restore.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
$(CC) $(CFLAGS) pg_restore.o $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
-pg_dumpall: pg_dumpall.o dumputils.o | submake-libpq submake-libpgport submake-libpgfeutils
+pg_dumpall: pg_dumpall.o dumputils.o | submake-libpq submake-libpgport submake-libpgfeutils submake-generated-headers
$(CC) $(CFLAGS) pg_dumpall.o dumputils.o $(WIN32RES) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
install: all installdirs
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 7f5bb1343e..0c4d648969 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -14,6 +14,7 @@
*/
#include "postgres_fe.h"
+#include "common/check_listvars.h"
#include "dumputils.h"
#include "fe_utils/string_utils.h"
@@ -888,10 +889,8 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
/*
* Some GUC variable names are 'LIST' type and hence must not be quoted.
- * XXX this list is incomplete ...
*/
- if (pg_strcasecmp(mine, "DateStyle") == 0
- || pg_strcasecmp(mine, "search_path") == 0)
+ if (IsConfigOptionAList(mine))
appendPQExpBufferStr(buf, pos);
else
appendStringLiteralConn(buf, pos, conn);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 566cbf2cda..6446f0ced7 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -53,6 +53,7 @@
#include "catalog/pg_proc.h"
#include "catalog/pg_trigger.h"
#include "catalog/pg_type.h"
+#include "common/check_listvars.h"
#include "libpq/libpq-fs.h"
#include "dumputils.h"
@@ -11880,8 +11881,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
* Some GUC variable names are 'LIST' type and hence must not be
* quoted.
*/
- if (pg_strcasecmp(configitem, "DateStyle") == 0
- || pg_strcasecmp(configitem, "search_path") == 0)
+ if (IsConfigOptionAList(configitem))
appendPQExpBufferStr(q, pos);
else
appendStringLiteralAH(q, pos, fout);
diff --git a/src/include/Makefile b/src/include/Makefile
index a689d352b6..a05f4ca197 100644
--- a/src/include/Makefile
+++ b/src/include/Makefile
@@ -13,7 +13,7 @@ top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-all: pg_config.h pg_config_ext.h pg_config_os.h
+all: pg_config.h pg_config_ext.h pg_config_os.h common/check_listvars.h
# Subdirectories containing installable headers
@@ -25,6 +25,9 @@ SUBDIRS = access bootstrap catalog commands common datatype \
port/win32_msvc/sys port/win32/arpa port/win32/netinet \
port/win32/sys portability
+common/check_listvars.h:
+ ./gen_listvars.pl
+
# Install all headers
install: all installdirs
# These headers are needed by the public headers of the interfaces.
@@ -73,7 +76,7 @@ uninstall:
clean:
- rm -f utils/fmgroids.h utils/fmgrprotos.h utils/errcodes.h parser/gram.h utils/probes.h catalog/schemapg.h
+ rm -f utils/fmgroids.h utils/fmgrprotos.h utils/errcodes.h parser/gram.h utils/probes.h catalog/schemapg.h common/check_listvars.h
distclean maintainer-clean: clean
rm -f pg_config.h pg_config_ext.h pg_config_os.h dynloader.h stamp-h stamp-ext-h
diff --git a/src/include/gen_listvars.pl b/src/include/gen_listvars.pl
new file mode 100755
index 0000000000..081d2ea855
--- /dev/null
+++ b/src/include/gen_listvars.pl
@@ -0,0 +1,145 @@
+#! /usr/bin/perl
+# src/include/gen_listvars.pl
+# generates a function definition to check whether a guc variable is
+# of list type or not.
+use strict;
+use File::Find;
+
+my $output_file = "common/check_listvars.h";
+my @varlist;
+
+find(\&findproc, "../");
+
+open my $out, '>', $output_file ||
+ die "failed to open output file $output_file: $!";
+print $out make_func_def(@varlist);
+close($out);
+print "done. The definition is written as $output_file.\n";
+exit;
+
+###############################################
+# findproc()
+# callback function for find() to retrieve names of list variables
+# from a file. The names are stored to @varlist.
+sub findproc
+{
+ my $f = $_;
+ my @l;
+ my $prepend = 0;
+
+ # we are interested only in .c files.
+ return if ($f !~ /\.c$/);
+
+ my $content = load_file($f);
+ if ($f =~ /^guc.c$/)
+ {
+ # guc varialbes are listed first
+ @l = extract_gucdef($content);
+ $prepend = 1;
+ }
+ else
+ {
+ # others follow guc variables
+ @l = extract_customdef($content);
+ }
+
+ @l = sort @l;
+
+ if ($#l >= 0)
+ {
+ printf("%d list variables found in %s\n", $#l + 1, $f);
+ }
+
+ if ($prepend)
+ {
+ @varlist = (@l, @varlist);
+ }
+ else
+ {
+ @varlist = (@varlist, @l);
+ }
+}
+
+###############################################
+# extract_gucdef($filecontent);
+# collects the the name of a enbedded GUC variable with GUC_LIST_INPUT
+sub extract_gucdef
+{
+ my ($str) = @_;
+ my @ret = ();
+
+ while ($str =~ /{"([^"]+)" *, *PGC_[^}]*\s*GUC_LIST_INPUT\s*[^}]*},/g)
+ {
+ push (@ret, $1);
+ }
+
+ return @ret;
+}
+
+##################################################################
+# extract_customdef($filecontent);
+# collects the the name of a custom variable with GUC_LIST_INPUT
+sub extract_customdef
+{
+ my ($str) = @_;
+ my @ret = ();
+
+ while ($str =~ /DefineCustomStringVariable\("([^"]+)"\s*,(?:(?!\)\s*;).)*,\s*GUC_LIST_INPUT\s*,(?:(?!\)\s*;).)*\);/g)
+ {
+ push (@ret, $1);
+ }
+
+ return @ret;
+}
+
+
+########################################################################
+# load_file($filename);
+# returns the file content as a string in which line-feeds are removed
+sub load_file
+{
+ my $f = $_[0];
+ my $ret = "";
+
+ open my $in, '<', $f || die "failed to open file $f: $!";
+ $ret = "";
+ while (<$in>)
+ {
+ chomp;
+ $ret .= $_;
+ }
+ close $in;
+
+ return $ret;
+}
+
+###############################################
+# make_func_def(@varnames);
+# generates the function definition
+sub make_func_def
+{
+ my @list = @_;
+ my $ret;
+
+ $ret =
+ "/* $output_file. Generated by src/include/gen_listvars.pl */\n".
+ "/* \n".
+ " * This is generated automatically, but not maintained automatically.\n".
+ " * make clean will remove file and generated in the next build.\n".
+ "*/\n\n".
+ "inline bool\n".
+ "IsConfigOptionAList(const char *name)\n".
+ "{\n".
+ " if (";
+ my $first = 1;
+
+ foreach my $i (@list)
+ {
+ $ret .= " ||\n " if (!$first);
+ $first = 0;
+ $ret .= "pg_strcasecmp(name, \"$i\") == 0";
+ }
+ $ret .= ")\n return true;\n return false;\n}\n";
+
+ return $ret;
+}
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index d7eff6c0a7..ce79515ab6 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3228,6 +3228,30 @@ SELECT pg_get_partkeydef(0);
(1 row)
+-- test for pg_get_functiondef with list parameters which should not be
+-- quoted.
+CREATE FUNCTION func_with_set_params() RETURNS integer
+ AS 'select 1;'
+ LANGUAGE SQL
+ SET search_path TO 'pg_catalog'
+ SET wal_consistency_checking TO heap, heap2
+ SET session_preload_libraries TO foo, bar
+ IMMUTABLE STRICT;
+SELECT pg_get_functiondef('func_with_set_params'::regproc);
+ pg_get_functiondef
+----------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.func_with_set_params()+
+ RETURNS integer +
+ LANGUAGE sql +
+ IMMUTABLE STRICT +
+ SET search_path TO pg_catalog +
+ SET wal_consistency_checking TO heap, heap2 +
+ SET session_preload_libraries TO foo, bar +
+ AS $function$select 1;$function$ +
+
+(1 row)
+
+DROP FUNCTION func_with_set_params();
-- test rename for a rule defined on a partitioned table
CREATE TABLE parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1);
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 0823c02acf..0d4938d715 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1170,6 +1170,18 @@ SELECT pg_get_function_arg_default(0, 0);
SELECT pg_get_function_arg_default('pg_class'::regclass, 0);
SELECT pg_get_partkeydef(0);
+-- test for pg_get_functiondef with list parameters which should not be
+-- quoted.
+CREATE FUNCTION func_with_set_params() RETURNS integer
+ AS 'select 1;'
+ LANGUAGE SQL
+ SET search_path TO 'pg_catalog'
+ SET wal_consistency_checking TO heap, heap2
+ SET session_preload_libraries TO foo, bar
+ IMMUTABLE STRICT;
+SELECT pg_get_functiondef('func_with_set_params'::regproc);
+DROP FUNCTION func_with_set_params();
+
-- test rename for a rule defined on a partitioned table
CREATE TABLE parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1);
--
2.16.2
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?
regards, tom lane
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.
As the discussion upthread, with the dynamic (or on the fly)
approach, pg_dump fails when required extension is not
loaded. Especially plpgsql variables are the candidate stopper of
ordinary pg_dump operation. We might have to implicitly load the
module by any means to make it work. If we treat extensions
properly, we must find the extension that have defined a GUC that
is about to be exported, then load it.
I don't think the automatic stuff is essential but the
check_listvars.h is still effective to reduce the effort needed
to maintain the multiple lists that should have the same set of
names of the list-gucs.
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...
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
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.
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.
--
Michael
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
Attachments:
0001-Store-whether-elements-of-proconfig-are-list-or-not-.patchtext/x-patch; charset=us-asciiDownload
From d48def7a489046e44755bef250f6b35d78339803 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Thu, 15 Mar 2018 18:21:50 +0900
Subject: [PATCH 1/2] Store whether elements of proconfig are list or not in
pg_proc.
---
src/backend/catalog/pg_aggregate.c | 1 +
src/backend/catalog/pg_proc.c | 5 +++
src/backend/commands/functioncmds.c | 77 ++++++++++++++++++++++++++++++++++++-
src/backend/commands/proclang.c | 3 ++
src/backend/commands/typecmds.c | 1 +
src/backend/utils/misc/guc.c | 24 ++++++++++++
src/include/catalog/pg_class.h | 2 +-
src/include/catalog/pg_proc_fn.h | 1 +
src/include/utils/guc.h | 1 +
9 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index 50d8d81f2c..3aaf0eac15 100644
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -631,6 +631,7 @@ AggregateCreate(const char *aggName,
parameterDefaults, /* parameterDefaults */
PointerGetDatum(NULL), /* trftypes */
PointerGetDatum(NULL), /* proconfig */
+ PointerGetDatum(NULL), /* proconfigislist */
1, /* procost */
0); /* prorows */
procOid = myself.objectId;
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 40e579f95d..cab0a225bd 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -87,6 +87,7 @@ ProcedureCreate(const char *procedureName,
List *parameterDefaults,
Datum trftypes,
Datum proconfig,
+ Datum proconfigislist,
float4 procost,
float4 prorows)
{
@@ -374,6 +375,10 @@ ProcedureCreate(const char *procedureName,
values[Anum_pg_proc_proconfig - 1] = proconfig;
else
nulls[Anum_pg_proc_proconfig - 1] = true;
+ if (proconfigislist != PointerGetDatum(NULL))
+ values[Anum_pg_proc_proconfigislist - 1] = proconfigislist;
+ else
+ nulls[Anum_pg_proc_proconfigislist - 1] = true;
/* proacl will be determined later */
rel = heap_open(ProcedureRelationId, RowExclusiveLock);
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index b1f87d056e..d6da508eb3 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -631,6 +631,73 @@ update_proconfig_value(ArrayType *a, List *set_items)
return a;
}
+static ArrayType *
+make_proconfig_islist(ArrayType *proconfig)
+{
+ ArrayType *retarray = NULL;
+ int index;
+ int i;
+ bool isnull;
+ int16 bool_typlen;
+ bool bool_typbyval;
+ bool bool_typalign;
+
+ get_typlenbyvalalign(BOOLOID,
+ &bool_typlen, &bool_typbyval, &bool_typalign);
+
+ index = ARR_DIMS(proconfig)[0];
+
+ for (i = 1 ; i <= index ; i++)
+ {
+ Datum d;
+ char *name;
+ bool is_list = false;
+
+ d = array_ref(proconfig, 1, &i,
+ -1 /* varlenarray */ ,
+ -1 /* TEXT's typlen */ ,
+ false /* TEXT's typbyval */ ,
+ 'i' /* TEXT's typalign */ ,
+ &isnull);
+ if (!isnull)
+ {
+ char *p;
+ int flags;
+
+ name = TextDatumGetCString(d);
+ p = strchr(name, '=');
+ if (p == NULL)
+ elog(ERROR, "something's wrong?");
+ *p = 0;
+
+ flags = GetConfigOptionFlags(name, false);
+ if (flags & GUC_CUSTOM_PLACEHOLDER)
+ ereport(ERROR,
+ (errmsg ("the module for variable \"%s\" is not loaded yet",
+ name),
+ errdetail ("The module must be loaded before referring this variable.")));
+ if (flags & GUC_LIST_INPUT)
+ is_list = true;
+ }
+
+ if (retarray)
+ retarray = array_set(retarray, 1, &i,
+ BoolGetDatum(is_list), false,
+ -1,
+ bool_typlen, bool_typbyval, bool_typalign);
+ else
+ {
+ Datum booldatum = BoolGetDatum(is_list);
+ retarray =
+ construct_array(&booldatum, 1,
+ BOOLOID,
+ bool_typlen, bool_typbyval, bool_typalign);
+ }
+ }
+
+ return retarray;
+}
+
/*
* Dissect the list of options assembled in gram.y into function
@@ -649,6 +716,7 @@ compute_function_attributes(ParseState *pstate,
bool *security_definer,
bool *leakproof_p,
ArrayType **proconfig,
+ ArrayType **proconfigislist,
float4 *procost,
float4 *prorows,
char *parallel_p)
@@ -767,7 +835,10 @@ compute_function_attributes(ParseState *pstate,
if (leakproof_item)
*leakproof_p = intVal(leakproof_item->arg);
if (set_items)
+ {
*proconfig = update_proconfig_value(NULL, set_items);
+ *proconfigislist = make_proconfig_islist(*proconfig);
+ }
if (cost_item)
{
*procost = defGetNumeric(cost_item);
@@ -887,6 +958,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
isLeakProof;
char volatility;
ArrayType *proconfig;
+ ArrayType *proconfigislist;
float4 procost;
float4 prorows;
HeapTuple languageTuple;
@@ -911,6 +983,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
isLeakProof = false;
volatility = PROVOLATILE_VOLATILE;
proconfig = NULL;
+ proconfigislist = NULL;
procost = -1; /* indicates not set */
prorows = -1; /* indicates not set */
parallel = PROPARALLEL_UNSAFE;
@@ -922,7 +995,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
&as_clause, &language, &transformDefElem,
&isWindowFunc, &volatility,
&isStrict, &security, &isLeakProof,
- &proconfig, &procost, &prorows, ¶llel);
+ &proconfig, &proconfigislist,
+ &procost, &prorows, ¶llel);
/* Look up the language and validate permissions */
languageTuple = SearchSysCache1(LANGNAME, PointerGetDatum(language));
@@ -1113,6 +1187,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
parameterDefaults,
PointerGetDatum(trftypes),
PointerGetDatum(proconfig),
+ PointerGetDatum(proconfigislist),
procost,
prorows);
}
diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c
index 447bd49f89..2fcba890fe 100644
--- a/src/backend/commands/proclang.c
+++ b/src/backend/commands/proclang.c
@@ -142,6 +142,7 @@ CreateProceduralLanguage(CreatePLangStmt *stmt)
NIL,
PointerGetDatum(NULL),
PointerGetDatum(NULL),
+ PointerGetDatum(NULL),
1,
0);
handlerOid = tmpAddr.objectId;
@@ -181,6 +182,7 @@ CreateProceduralLanguage(CreatePLangStmt *stmt)
NIL,
PointerGetDatum(NULL),
PointerGetDatum(NULL),
+ PointerGetDatum(NULL),
1,
0);
inlineOid = tmpAddr.objectId;
@@ -223,6 +225,7 @@ CreateProceduralLanguage(CreatePLangStmt *stmt)
NIL,
PointerGetDatum(NULL),
PointerGetDatum(NULL),
+ PointerGetDatum(NULL),
1,
0);
valOid = tmpAddr.objectId;
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index bf3cd3a454..9cbd98fbca 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1685,6 +1685,7 @@ makeRangeConstructors(const char *name, Oid namespace,
NIL, /* parameterDefaults */
PointerGetDatum(NULL), /* trftypes */
PointerGetDatum(NULL), /* proconfig */
+ PointerGetDatum(NULL), /* proconfigislist */
1.0, /* procost */
0.0); /* prorows */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a4f9b3668e..c67f68dc1b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8112,6 +8112,30 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok)
return _ShowOption(record, true);
}
+/*
+ * Return "flags" for a given GUC variable. If missing_ok is true, ignore
+ * any errors and return 0 to the caller instead.
+ */
+int
+GetConfigOptionFlags(const char *name, bool missing_ok)
+{
+ struct config_generic *record;
+
+ record = find_option(name, false, ERROR);
+
+ if (record == NULL)
+ {
+ if (missing_ok)
+ return 0;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ }
+
+ return record->flags;
+}
+
/*
* Return GUC variable value by variable number; optionally return canonical
* form of name. Return value is palloc'd.
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 01cf59e7a3..26b1866c69 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -151,7 +151,7 @@ DATA(insert OID = 1247 ( pg_type PGNSP 71 0 PGUID 0 0 0 0 0 0 0 f f p r 30 0 t
DESCR("");
DATA(insert OID = 1249 ( pg_attribute PGNSP 75 0 PGUID 0 0 0 0 0 0 0 f f p r 22 0 f f f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
-DATA(insert OID = 1255 ( pg_proc PGNSP 81 0 PGUID 0 0 0 0 0 0 0 f f p r 28 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
+DATA(insert OID = 1255 ( pg_proc PGNSP 81 0 PGUID 0 0 0 0 0 0 0 f f p r 29 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
DATA(insert OID = 1259 ( pg_class PGNSP 83 0 PGUID 0 0 0 0 0 0 0 f f p r 33 0 t f f f f f f t n f 3 1 _null_ _null_ _null_));
DESCR("");
diff --git a/src/include/catalog/pg_proc_fn.h b/src/include/catalog/pg_proc_fn.h
index b66871bc46..94dadc5c7b 100644
--- a/src/include/catalog/pg_proc_fn.h
+++ b/src/include/catalog/pg_proc_fn.h
@@ -40,6 +40,7 @@ extern ObjectAddress ProcedureCreate(const char *procedureName,
List *parameterDefaults,
Datum trftypes,
Datum proconfig,
+ Datum proconfigislist,
float4 procost,
float4 prorows);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 2e03640c0b..7951257c50 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -368,6 +368,7 @@ extern int set_config_option(const char *name, const char *value,
extern void AlterSystemSetConfigFile(AlterSystemStmt *setstmt);
extern char *GetConfigOptionByName(const char *name, const char **varname,
bool missing_ok);
+extern int GetConfigOptionFlags(const char *name, bool missing_ok);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
--
2.16.2
0002-pg_proc.h-part.patch.gzapplication/octet-streamDownload
�K@�Z 0002-pg_proc.h-part.patch ��ks���-�Y�����n[�M |�>�>v�����=n;�9q�DB�$�@�����~�� �V�����%m�@�\�������,�;��7}�����t��On�G�s����s����t��K���4q����p&����o2q�O�5�q��--�,u�g�����%v��K����������IQ�z����������E���/�G��9�f��n���o<�7�7��,'��}|�k�.~��?�����������:������_�8�0+~���?�����Wq��7�W��w�������3�/������c����q��x9��0�F�G�����<��8M���R�n�l�]$�����l??;?n�� ���4�����&��L�7OS�y�/��h�F�`2�9�d2�N��:9?c*je����|\L��������������w���g]o6����������������_��������oV��wW~�����o�����~��?��g�C}.�[�����~���������������/.#�����c���������?��zx����u�l����\�<���u�8O����������x���������t����o�k�"N�9�W���.��������oRn���E�o`?����z��s��}���q����(a��3�o�?�����pUR���u�������N�v�6�s���/�o�'���t`��9L���"w���jF�3N��9����'���(�������g����)T����X�_�p��������}�S*��D{�/�q]������ "H�z��B���#��G���H��Z4l��y�9����.8���]� ?s��7��C��E�������?���;6���l����Ng�'�S!���Y��,�����w,�|�!����/��M=���<�����8�������}�����L��?��/��b'�u�����}����n��&����/���~E���:���������~��/�?���+����s����1���IH�T@~+�v��RJ(t�����m}`auQ*�@�q�,O�`' '�P}l��^[���q�� �S(���L ����� �I F��lk��1`�7)���8�8)<����_J �$�
#sY�4���}�8����B����w�j�*)��Pb����cX��b�]�N�d�?�������]�02�
�m���nJF����h�g�L�&y�y
x@-��
@��Y��/���0E`��J"B�?]
�(�U�������y?D+x]J�i��q��
�
�]�L/����u@o�Y($W�0��!E��QF���,�\���~RB`����l�?��2p�bx^������ r���0"�%\ay��M�K ��Ad��p���������W�?2�H��kmy�y��^IF�h�ml����-����aD�%\k�����m�� 6�L1XI�����?���J#F������?���>K
%F���H��7����9rc���f7�k/�En{{p[�|R��~�9��G�[���������ap^��kn� +�0�j���������V��Z+m��Y5so��� S�0s�
�s8��z������[���0�[��@h�?��%�M�%0o�2���m��'�(��������C20�� q^��i��.����Wc�=s;�9gQo��K� ph�����n��enQ�����H��Y���B~.�)!R���!���5)d���v
D
��"�^�B&��H!��@���k �n?�F�������a�$��A�4r~
�2
D
��"�Q����|���s|�G���_��Q���x���H���*�}wdjf9��Y�,g����#����4���g�M�h%���zK���� ��w��E�P��<rM��.�\b�%���\Z����S�]�G�������R��d!��+����J����S��R����EylT��!����+6C�2P���jo�L�9B�a4GFx'Q�e*����1�II���6�q�*e`A�w�
�
�����4/�Y���}W�s�"���2
����]m6��:���d��[��t!f;� � ��k�.�`������n�2;>���QN/Dz��D�_Q� ���{$a�������i�G��\^��s��t�p����M&�G��i7����y�3�3�����f1q�+9�����fP�0,����K}�u]��5�E����x��
�����d�.,����,,�,W��9BU���"�pB6���4�������O�s�G��%4Q����m��O
A�&,B^�Sm�i��vl�v#����mAv��p����~g^f�4��q�S-Gq�v����R�2�������c63�m-��@`4��c����V�B<���;���4�`73$�����|���h��,B�G/�J*�7�
�~+ m�&bka8�x�\�
�N<��z��&x�v�^�
�\����bckg�{�n�
���_u'b����������b�����@&��k����;i�����#����0"�����N���uY��p
�d1��.�
1<��`)7%�-r���������)}�V����a�3\�o�I�:��>E���@+A���1��@/\j�j=V$��3+�S�O�J7���D�aa5h@[VVd����Ptb��f�G[g@�g(E���@�Mj��n`��W20J���H�!,^,� ��%E���@�������p�J)��>�T�JU�*�<EA.Y��;)p��
����|7@R@�Zg�,�v����t)G�l/�:�78�y9F�T��-jxH}PkxX��*x��/��g[V��&\,������Xr���2��j�b�3�����E���;�\��oPf���|������60���0�������q
�h��K�i�:<Y5��q�2@p�
OHm�����a���������3G��=4B@��s �(��o]��]JS�?�H*���`i�c�4�2
H��$�m$�<M�V��K*g�`���i����4`�dq�r9
D2�'��N�Zv���mU|N3��:��,������������D�x��ZR�?�2i�]�u6��Y���z�?zf]_�Gk�2j������N�jc`���Q����g*F��j�zV�1p�D��.h2+.�k����,��
Q���}�@�������}���A���08��GaG�Y]|����������Nb�nnn�t� H/��8���atu99��}�v�����ZU�6��+ ���������M���������d5���:�Sb�
����*D�tc�����Cb�Y�������G�����C#Jss�g��F7o����CS���i���F����s�&�����B�Y���+�f(0���x���3( }8������1��y����;����{�j`� �5n�ppp��k����
F�D=+t�����:����{�>�5s�08|���k�Gw��E�D;+t`g��������i�xf��S��U7�����
@���qPo2���6��|G�"�w������K9 �z9�v�5�w����4�r��o0���
�v� ��b�����J�7�i��;k@�f�&���K���P8vX�����R��]r'��,�)1RX��@����Ha=Z����"��i
D
+��I�Sk RX��@$�f��Hb�Z�����#�Ul
Fk��V�5I�kk0�X��Hc�[���J�#��n
F���$��5I��k0�X�`$���HbM\�����0F*���DV�5(���kPY1����n��Ha�\���#���a�$��5)��k �X[�`$�������������Hf�]�����,��w
Nj���p ��k Z�� ��.�Lqu���_~)+klWI��VEE��q�w9���o�����^rH[����.���B�1~Y��i�����R�����(`v�|~�-���
�&�#<��`v�|����a:b@� bU����������7�8o�t~�U4��`�=�����7?!�hT2 ���C|���
h������J!F����g��;�/��j9jn{A�
HxJ�$z���L��<�%F���'�/� N���;"��������Et��� �E/|�o���
�U�^3�����X���b�H�C&���D�^��"�*�8�J'i���I�T"�&���M���6��a9E&y,�6>Q�Q�J-�����.-%����� �O�L��TE��J��}����I)
#�"�.�����G�n�T)""6����k�h���{�qdb)�x�w�������������Mn�s������D�u������a������k! �db�6yc`1�qj��C-�M�X�l�@�s+n�5���
#:��.[��G3v��V�f�7��1�q#z�D���)���������.� ��fn�T����"�*�8�J'
%�4 �D&
L����0Y��,Z�l�N�O�� �R;�Q���`���LR=������V3HV!�f���.�Fs]^kli7d���\E�]�M������<�V� !����L��5���v1~c�=F���b�{����
��4B@�%dkMoO��������>*2�M��
8
%zR���u���p�:I�K��o��3e?&�&�"c)�M�@� `�2,>,�TG�/�W�Q����s���K���:���8���)�cU.!N���}hW�W,K�L.�n�#B���M�����r'��mrS� �����V|��4+z����
�8�f��������� �����O�lxH�e���T�&�|�)i��h�yo?kH�}oo��#�s���y=�KX�p��G����b�����|�R���f�n�����������e��Y\������>��7���C������������C�{��hI�Sr���k0�4x�}wd��N��V�<��d��O�"�=��w���U�j9�����7�2m��;���:��*f~��9Zm���u��
s/p���f��@� bm����T���S^���tkd^�4�Y�X���������7GV����f����ZB1r��!�vv>�>Q�%�#�o����
���ZB1r������(��b�AJQ��oV��+�Y�C����T
>����;��bM�B������I���g��$bj
JJ����9�R6A��HP[���`bW#���p4��f�[r���<��u(���o��C[
�����R�v��Q�
n�B���N&���O����I�
[�x=��������eQR8��"��[����rl�����967�H��:�E���V�|��������V�|������S��-�6��X`T�L��8k�8��U�U(1��<��Qu���5�k��i��1-�v����e�]��1��Z�+U>l��a�h���JU
�ck�]��Sv�����$x��9��N���I �� �5^��
�����@�u�)x�|~:���H���Lt��N��Lvq������!^^;�~�NW��z`�X�<�,�������}T'�����TMp.���h|������i��+&�'Z���0X~S"%4� %4$� %40� %4<� %4H� %4T� %4`� %4lFJi��AJh�AJh �AJo8�LkP�Kkh�Kq���Lq���Lq���Lq�=���[���[�� \���P��Z-a����N����V��,}��of������7����������.���>2��Y���"w6�,N���Y(*i�J�~=���$.�an3�C�u+)4!�]{��.�o�BN���Q������bH���D-EMt/��A�{�@�Ou��J
�
d8��g2L�������k�N�RiS�Pt��7��N+.��Y���
�Dw�^��v�Yq�����V�D�������u~9\��5hs7�L�/[��-�e�C ���mr�������7���������K)vj�R������,�h�K)�j�R��
���4h�%g��g|KU-��Z��������PA��5��0!��i����V�`� ��Vd���"D7e-[����������5�L���$���q�i��d��'�������|�������Q��d��u!�{/[��]+G��w+��k�0B����Nv-E�������4�����eMt$��58��k
N2�[��L��$����w{v�>a�N��^6 al��|�K�{�g�����/�������~�� f!��=k?��0�y�?iO��40�(�~�� ��"�d��8��A)��o�OG�AP�����D�}*a��u����D�1��:hc�G�]�n9��<�1�����1��#����1��M��z:�����O�o<�����<�M3my���1������,�)�~\��Po�AJ�O� %��k���5H �����z
RB=�)�~)��)���^m��9�q�$��MQ�7����xw����8/�m����}�u���
1��2�;V ���WA�S�o��VJas����;�-+�i�
�����_nsG5�����x
r�����V��plJV�%z�6������e��q
���c�w�E3�P��h���[M�]X�8�P&n6B#�;�2���� ����\��������]ZyG���
���
�$���V.}}�l��N�^Gy����������,�a}���E���sF�k�o}~�������Y���-��,��J�U//K} ��J���rNa�v���"������uj� Y'���:��u_�����6"@�� ��<��\�����5"@�� �R���ut1q��5�t�~*��t2{�������|'|���0��VI�i���Q���^[C
�������Y�>��[�7�t��$r�u�8�=N>�"�n#U�)�����B^4�H_��������J�lE�u�4��i&!���kp�w%"���*��&r�;�+���p���������/.ys�����sH��r]�5 �jT�����y�dz.od?lW��
�+6Y2�x�f�X�Z����g�� 3������eb�C�3�UO��asy�{��LcPe0�/�y����� N�\���>9��HS�DS�P�A����M�����>"):������kQl��
��b���(ou���
�v�e)��@w���e���b@�^�1}�2���0+I T�jo_���^�U���Nk����o�rv�����)mDa`���F��� ]�@Yl���8@���KP�F*�7]WRr�^�P�� ��5�)�~
dR�+�n@�Zg��K�Kh0V#�o���j^W��.?������ �8���RJ���������w���,,��f�1�LY�����K�.���r/�6��B���z<���4+�����A>�/��H�H'
�k��S>��^�(DL�<�7=nP}���|\���Fs������k����������&��P��nB/�q�r�`4� .�q�p-��\w-��T�C�!%�Zr�`N�e�� ��"�q��,���B���Y��;w2/-��z=-��y����Ha�+�z��-�~.�)�7v���1��Q� �����|*����9z8A�V�=�Lk�y��E�q���/�i�p"a�"�K<^��T����R���������X�2��0�
�n��f���B@�� ��g���HB�1�K�]jQ
y&���~s&�".`���\�s��k�����"�\�%^�!�t^���L:
0���K�H'
������0YY9�b9q�����`t�Ax��eK���}���`���M�E��k]���6C
ys��z��(�y�`/b�\� �,|�����������O��O]�O��<�e���)����/���k6iT [�����2��%>��&��������7���[\��/O�*6��Y���"��b�%�a5{(C���k(�^�V��2�m!6�\���f��=�E���p���m��lb]������b�
���Qt!_j���o����nr���R�;@~�e����r��E�M�,C����,0�\��\�����x]}G�s��m�"b.�|vy�]mb^�fAs�,��}��>�6I/V��*f����93��"G����C�M���N�Q�R@����5@�j������t1�9
Pl�Z��<�m\z���+1 �E"Hb���b�@D��j�D��b?M��-�9[Y���-�)��x�(|{�����Q V��9�5�a�i#������haoc�2@p�5Q�^%��H&M����\Z��K%�����bh`RI440���t�
P:��(�d"�4D��V2�K!%�@$��48[�O'�������-W��tC/��9_����'dE�](�� �����)|����K~}
��d:�/��l4����"P~���`���]��&������{p�4� V�`��~v���]a�.V��<1q�����"R
J1�z/.������b��V�L�Q<�|����N�&� y"z��a��J �3
p]���U^S�
���(��Rn��8'r �l�xG�/E���TV������e�� �_�0>�Y��K��KL���$X���I�,\���N���^��K�]���/E�`������T�6Zs��%�X&��d�we5��m���N �8z3U��G{'jX�=��>���� =zz���� P�y&
�h��A��fo,zq�-k��AW~h��Z������C�a�K+��u���d�c��|
p�����-N7@<�D�����[�����v��J ������hC�u�%�'�0���{RO`�e1�6[��`�� �o��l;�7�=�#�_�`��[w!�a���w��8-!`&�bw�������8|�I��!l�������������<z7)���;���������0'������p���h�g��lwf[
�81���/������k��KW���qc~��=h������g>+e`NN"���p;?a�79��97]�X���8#�!Z07w.�Mh��y��l9(c��5��nI-��~�%��>��#+�+�J�Pb>L�������qW���qc9�w��������g[��� z�{�r9y1����V�i�V���������xPy~����R��3����|�������Shu��jQ`,'����:=&�S�D�S�9�����|��f[��"W��Oj��D�7�v�0�$��Y���c�"��B^"����#���h�3�~K
fR �6(?��/>��)�e�C�w��+Q����Kv����~���R�-���w,S� W�K ��u�U�F?�5=��,/{7��K ��atz������x������S��a�4���K �in��>��jp����Q���M�zktL�nQ��~������E?���
\�?�y)�+�5��A��;���K���"��E��mon���{��������;����m�����{5�m/>
}�"��L�D��H�W�`���i R��4)�s�I�v��<
D
=_�naQ��)o�6 �]q`�PG�i���1:4c]�1{����/F��u��no��Y����[NZ�
w�G�f�4��&��h��$��#�V��H�-k0�h���8��):5/S"