[PATCH] two-arg current_setting() with fallback
Apologies if this is a double-post.
Enclosed is a patch that creates a two-arg current_setting() function. From the commit message:
The two-arg form of the current_setting() function will allow a
fallback value to be returned instead of throwing an error when an
unknown GUC is provided. This would come in most useful when using
custom GUCs; e.g.:
-- errors out if the 'foo.bar' setting is unset
SELECT current_setting('foo.bar');
-- returns current setting of foo.bar, or 'default' if not set
SELECT current_setting('foo.bar', 'default')
This would save you having to wrap the use of the function in an
exception block just to catch and utilize a default setting value
within a function.
Attachments:
0001-Add-two-arg-for-of-current_setting-NAME-FALLBACK.patchapplication/octet-stream; name=0001-Add-two-arg-for-of-current_setting-NAME-FALLBACK.patchDownload
>From 2f1a7345e31c62c5af942d1e418655b59c15213d Mon Sep 17 00:00:00 2001
From: David Christensen <david@endpoint.com>
Date: Thu, 19 Mar 2015 17:35:01 -0500
Subject: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)
The two-arg form of the current_setting() function will allow a
fallback value to be returned instead of throwing an error when an
unknown GUC is provided. This would come in most useful when using
custom GUCs; e.g.:
-- errors out if the 'foo.bar' setting is unset
SELECT current_setting('foo.bar');
-- returns current setting of foo.bar, or 'default' if not set
SELECT current_setting('foo.bar', 'default')
This would save you having to wrap the use of the function in an
exception block just to catch and utilize a default setting value
within a function.
---
src/backend/utils/misc/guc.c | 50 ++++++++++++++++++++++++++++++++++++---
src/include/catalog/pg_proc.h | 2 ++
src/include/utils/builtins.h | 1 +
src/include/utils/guc.h | 1 +
src/test/regress/expected/guc.out | 19 +++++++++++++++
src/test/regress/sql/guc.sql | 12 ++++++++++
6 files changed, 82 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 26275bd..002a926 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7703,13 +7703,32 @@ ShowAllGUCConfig(DestReceiver *dest)
char *
GetConfigOptionByName(const char *name, const char **varname)
{
+ return GetConfigOptionByNameFallback(name, NULL, varname);
+}
+
+/*
+ * Return GUC variable value by name; optionally return canonical form of
+ * name. If GUC is NULL then optionally return a fallback value instead of an
+ * error. Return value is palloc'd.
+ */
+char *
+GetConfigOptionByNameFallback(const char *name, const char *default_value, const char **varname)
+{
struct config_generic *record;
record = find_option(name, false, ERROR);
if (record == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("unrecognized configuration parameter \"%s\"", name)));
+ {
+ if (default_value) {
+ return pstrdup(default_value);
+ }
+ else
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter \"%s\"", name)));
+ }
+ }
if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -8008,6 +8027,31 @@ show_config_by_name(PG_FUNCTION_ARGS)
}
/*
+ * show_config_by_name_fallback - equiv to SHOW X command but implemented as
+ * a function. If X does not exist, return a fallback datum instead of erroring
+ */
+Datum
+show_config_by_name_fallback(PG_FUNCTION_ARGS)
+{
+ char *varname;
+ char *varfallback;
+ char *varval;
+
+ /* Get the GUC variable name */
+ varname = TextDatumGetCString(PG_GETARG_DATUM(0));
+
+ /* Get the fallback value */
+ varfallback = TextDatumGetCString(PG_GETARG_DATUM(1));
+
+ /* Get the value */
+ varval = GetConfigOptionByNameFallback(varname, varfallback, NULL);
+
+ /* Convert to text */
+ PG_RETURN_TEXT_P(cstring_to_text(varval));
+}
+
+
+/*
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
*/
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 6a757f3..71efed2 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3025,6 +3025,8 @@ DESCR("convert bitstring to int8");
DATA(insert OID = 2077 ( current_setting PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 25 "25" _null_ _null_ _null_ _null_ show_config_by_name _null_ _null_ _null_ ));
DESCR("SHOW X as a function");
+DATA(insert OID = 3280 ( current_setting PGNSP PGUID 12 1 0 0 0 f f f f t f s 2 0 25 "25 25" _null_ _null_ _null_ _null_ show_config_by_name_fallback _null_ _null_ _null_ ));
+DESCR("SHOW X as a function");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 0 f f f f f f v 3 0 25 "25 25 16" _null_ _null_ _null_ _null_ set_config_by_name _null_ _null_ _null_ ));
DESCR("SET X as a function");
DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t s 0 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,1009,25,25,25,23}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,boot_val,reset_val,sourcefile,sourceline}" _null_ show_all_settings _null_ _null_ _null_ ));
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index bc4517d..e3d9fbb 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -1088,6 +1088,7 @@ extern Datum quote_nullable(PG_FUNCTION_ARGS);
/* guc.c */
extern Datum show_config_by_name(PG_FUNCTION_ARGS);
+extern Datum show_config_by_name_fallback(PG_FUNCTION_ARGS);
extern Datum set_config_by_name(PG_FUNCTION_ARGS);
extern Datum show_all_settings(PG_FUNCTION_ARGS);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index d3100d1..145ad5d 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -352,6 +352,7 @@ extern int set_config_option(const char *name, const char *value,
bool is_reload);
extern void AlterSystemSetConfigFile(AlterSystemStmt *setstmt);
extern char *GetConfigOptionByName(const char *name, const char **varname);
+extern char *GetConfigOptionByNameFallback(const char *name, const char *fallback, const char **varname);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 4f0065c..905969b 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -736,3 +736,22 @@ NOTICE: text search configuration "no_such_config" does not exist
select func_with_bad_set();
ERROR: invalid value for parameter "default_text_search_config": "no_such_config"
reset check_function_bodies;
+-- check multi-arg custom current_setting
+-- this should error:
+select current_setting('nosuch.setting');
+ERROR: unrecognized configuration parameter "nosuch.setting"
+-- this should return 'fallback'
+select current_setting('nosuch.setting','fallback');
+ current_setting
+-----------------
+ fallback
+(1 row)
+
+-- this should return 'nada', not 'fallback'
+set nosuch.setting = 'nada';
+select current_setting('nosuch.setting','fallback');
+ current_setting
+-----------------
+ nada
+(1 row)
+
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 3de8a6b..48c0bed 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -275,3 +275,15 @@ set default_text_search_config = no_such_config;
select func_with_bad_set();
reset check_function_bodies;
+
+-- check multi-arg custom current_setting
+
+-- this should error:
+select current_setting('nosuch.setting');
+
+-- this should return 'fallback'
+select current_setting('nosuch.setting','fallback');
+
+-- this should return 'nada', not 'fallback'
+set nosuch.setting = 'nada';
+select current_setting('nosuch.setting','fallback');
--
2.3.3
David Christensen <david@endpoint.com> writes:
The two-arg form of the current_setting() function will allow a
fallback value to be returned instead of throwing an error when an
unknown GUC is provided. This would come in most useful when using
custom GUCs; e.g.:
-- errors out if the 'foo.bar' setting is unset
SELECT current_setting('foo.bar');
-- returns current setting of foo.bar, or 'default' if not set
SELECT current_setting('foo.bar', 'default')
This would save you having to wrap the use of the function in an
exception block just to catch and utilize a default setting value
within a function.
That seems kind of ugly, not least because it assumes that you know
a value that couldn't be mistaken for a valid value of the GUC.
(I realize that you are thinking of cases where you want to pretend
that the GUC has some valid value, but that's not the only use case.)
ISTM, since we don't allow GUCs to have null values, it'd be better to
define the variant function as returning NULL for no-such-GUC. Then the
behavior you want could be achieved by wrapping that in a COALESCE, but
the behavior of probing whether the GUC is set at all would be achieved
with an IS NULL test.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mar 19, 2015, at 6:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Christensen <david@endpoint.com> writes:
The two-arg form of the current_setting() function will allow a
fallback value to be returned instead of throwing an error when an
unknown GUC is provided. This would come in most useful when using
custom GUCs; e.g.:-- errors out if the 'foo.bar' setting is unset
SELECT current_setting('foo.bar');-- returns current setting of foo.bar, or 'default' if not set
SELECT current_setting('foo.bar', 'default')This would save you having to wrap the use of the function in an
exception block just to catch and utilize a default setting value
within a function.That seems kind of ugly, not least because it assumes that you know
a value that couldn't be mistaken for a valid value of the GUC.
(I realize that you are thinking of cases where you want to pretend
that the GUC has some valid value, but that's not the only use case.)ISTM, since we don't allow GUCs to have null values, it'd be better to
define the variant function as returning NULL for no-such-GUC. Then the
behavior you want could be achieved by wrapping that in a COALESCE, but
the behavior of probing whether the GUC is set at all would be achieved
with an IS NULL test.regards, tom lane
In that case, the other thought I had here is that we change the function signature of current_setting() to be a two-arg form where the second argument is a boolean "throw_error", with a default argument of true to preserve existing semantics, and returning NULL if that argument is false. However, I'm not sure if there are some issues with changing the signature of an existing function (e.g., with pg_upgrade, etc.).
My *impression* is that since pg_upgrade rebuilds the system tables for a new install it shouldn't be an issue, but not sure if having the same pg_proc OID with different values or an alternate pg_proc OID would cause issues down the line; anyone know if this is a dead-end?
Regards,
David
--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Mar 20, 2015 at 10:54 AM, David Christensen <david@endpoint.com> wrote:
In that case, the other thought I had here is that we change the function signature of current_setting() to be a two-arg form where the second argument is a boolean "throw_error", with a default argument of true to preserve existing semantics, and returning NULL if that argument is false. However, I'm not sure if there are some issues with changing the signature of an existing function (e.g., with pg_upgrade, etc.).
My *impression* is that since pg_upgrade rebuilds the system tables for a new install it shouldn't be an issue, but not sure if having the same pg_proc OID with different values or an alternate pg_proc OID would cause issues down the line; anyone know if this is a dead-end?
I think if the second argument is defaulted it would be OK. However
it might make sense to instead add a new two-argument function and
leave the existing one-argument function alone, because setting
default arguments for functions defined in pg_proc.h is kind of a
chore.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Mar 20, 2015 at 9:04 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Mar 20, 2015 at 10:54 AM, David Christensen <david@endpoint.com>
wrote:In that case, the other thought I had here is that we change the
function signature of current_setting() to be a two-arg form where the
second argument is a boolean "throw_error", with a default argument of true
to preserve existing semantics, and returning NULL if that argument is
false. However, I'm not sure if there are some issues with changing the
signature of an existing function (e.g., with pg_upgrade, etc.).My *impression* is that since pg_upgrade rebuilds the system tables for
a new install it shouldn't be an issue, but not sure if having the same
pg_proc OID with different values or an alternate pg_proc OID would cause
issues down the line; anyone know if this is a dead-end?I think if the second argument is defaulted it would be OK. However
it might make sense to instead add a new two-argument function and
leave the existing one-argument function alone, because setting
default arguments for functions defined in pg_proc.h is kind of a
chore.
Isn't there some other update along this whole error-vs-null choice going
around where a separate name was chosen for the new null-returning function
instead of adding a boolean switch argument?
David J.
On Mar 20, 2015, at 11:10 AM, David G. Johnston <david.g.johnston@gmail.com> wrote:
On Fri, Mar 20, 2015 at 9:04 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Mar 20, 2015 at 10:54 AM, David Christensen <david@endpoint.com> wrote:In that case, the other thought I had here is that we change the function signature of current_setting() to be a two-arg form where the second argument is a boolean "throw_error", with a default argument of true to preserve existing semantics, and returning NULL if that argument is false. However, I'm not sure if there are some issues with changing the signature of an existing function (e.g., with pg_upgrade, etc.).
My *impression* is that since pg_upgrade rebuilds the system tables for a new install it shouldn't be an issue, but not sure if having the same pg_proc OID with different values or an alternate pg_proc OID would cause issues down the line; anyone know if this is a dead-end?
I think if the second argument is defaulted it would be OK. However
it might make sense to instead add a new two-argument function and
leave the existing one-argument function alone, because setting
default arguments for functions defined in pg_proc.h is kind of a
chore.Isn't there some other update along this whole error-vs-null choice going around where a separate name was chosen for the new null-returning function instead of adding a boolean switch argument?
Well, speaking of the two-arg form vs alternate name, here's a version of the patch which includes the new behavior. (I couldn't think of a good name to expose for an alternate function, but I'm open to suggestions.)
Regards,
David
--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171
Attachments:
0001-Add-two-arg-form-of-current_setting-to-optionally-su.patchapplication/octet-stream; name=0001-Add-two-arg-form-of-current_setting-to-optionally-su.patchDownload
>From 5c0b6418971799e3d0b76a6ce55bff91a73fa371 Mon Sep 17 00:00:00 2001
From: David Christensen <david@endpoint.com>
Date: Fri, 20 Mar 2015 12:03:47 -0500
Subject: [PATCH] Add two-arg form of current_setting() to optionally suppress
errors
The current_setting() function by default throws an error if the GUC
is unset/unknown. Add a second argument "missing_ok" which will
return NULL when an unset GUC is encountered and the parameter is
true.
Since GUCs cannot be set to NULL, this can be used to check for the
existance of a particular GUC without throwing an error by testing
whether the two-arg form returns NULL or not.
This can also be combined with COALESCE() to provide support for a
fallback/default value instead of throwing an error when an unknown
GUC is encountered. This would come in most useful when using custom
GUCs; e.g.:
-- errors out if the 'foo.bar' setting is unset
SELECT current_setting('foo.bar');
-- returns current setting of foo.bar, or NULL if not set
SELECT current_setting('foo.bar', 'true')
-- returns current setting of foo.bar, or 'default' if not set
SELECT COALESCE(current_setting('foo.bar', 'true'), 'default')
This saves you having to wrap the use of the function in an exception
block just to catch and handle a default setting within a function.
---
doc/src/sgml/func.sgml | 6 ++++--
src/backend/utils/misc/guc.c | 43 ++++++++++++++++++++++++++++++++++++---
src/include/catalog/pg_proc.h | 2 ++
src/include/utils/builtins.h | 1 +
src/include/utils/guc.h | 4 +++-
src/test/regress/expected/guc.out | 36 ++++++++++++++++++++++++++++++++
src/test/regress/sql/guc.sql | 22 ++++++++++++++++++++
7 files changed, 108 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index aa19e10..d36a0f1 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16215,7 +16215,7 @@ SELECT collation for ('foo' COLLATE "de_DE");
<indexterm>
<primary>current_setting</primary>
</indexterm>
- <literal><function>current_setting(<parameter>setting_name</parameter>)</function></literal>
+ <literal><function>current_setting(<parameter>setting_name</parameter> [, <parameter>missing_ok</parameter> ])</function></literal>
</entry>
<entry><type>text</type></entry>
<entry>get current value of setting</entry>
@@ -16254,7 +16254,9 @@ SELECT collation for ('foo' COLLATE "de_DE");
The function <function>current_setting</function> yields the
current value of the setting <parameter>setting_name</parameter>.
It corresponds to the <acronym>SQL</acronym> command
- <command>SHOW</command>. An example:
+ <command>SHOW</command>. If <parameter>setting</parameter> does not exist, throws an error
+ unless <parameter>missing_ok</parameter> is <literal>true</literal>. An
+ example:
<programlisting>
SELECT current_setting('datestyle');
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 26275bd..5245ee8 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7696,20 +7696,27 @@ ShowAllGUCConfig(DestReceiver *dest)
end_tup_output(tstate);
}
+
/*
- * Return GUC variable value by name; optionally return canonical
- * form of name. Return value is palloc'd.
+ * Return GUC variable value by name; optionally return canonical form of
+ * name. If the GUC is unset, then throw an error unless missing_ok is true,
+ * in which case return NULL. Return value is palloc'd.
*/
char *
-GetConfigOptionByName(const char *name, const char **varname)
+GetConfigOptionByNameMissingOK(const char *name, const char **varname, bool missing_ok)
{
struct config_generic *record;
record = find_option(name, false, ERROR);
if (record == NULL)
+ {
+ if (missing_ok)
+ return NULL;
+
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("unrecognized configuration parameter \"%s\"", name)));
+ }
if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -8008,6 +8015,36 @@ show_config_by_name(PG_FUNCTION_ARGS)
}
/*
+ * show_config_by_name_missing_ok - equiv to SHOW X command but implemented as
+ * a function. If X does not exist, suppress the error and just return NULL
+ * if missing_ok is TRUE.
+ */
+Datum
+show_config_by_name_missing_ok(PG_FUNCTION_ARGS)
+{
+ char *varname;
+ bool missing_ok;
+ char *varval;
+
+ /* Get the GUC variable name */
+ varname = TextDatumGetCString(PG_GETARG_DATUM(0));
+
+ /* Get the missing_ok value */
+ missing_ok = PG_GETARG_BOOL(1);
+
+ /* Get the value */
+ varval = GetConfigOptionByNameMissingOK(varname, NULL, missing_ok);
+
+ /* return NULL if this was NULL */
+ if (!varval)
+ PG_RETURN_NULL();
+
+ /* Convert to text */
+ PG_RETURN_TEXT_P(cstring_to_text(varval));
+}
+
+
+/*
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
*/
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 3c218a3..d1092a0 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3040,6 +3040,8 @@ DESCR("convert bitstring to int8");
DATA(insert OID = 2077 ( current_setting PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 25 "25" _null_ _null_ _null_ _null_ show_config_by_name _null_ _null_ _null_ ));
DESCR("SHOW X as a function");
+DATA(insert OID = 3280 ( current_setting PGNSP PGUID 12 1 0 0 0 f f f f t f s 2 0 25 "25 16" _null_ _null_ _null_ _null_ show_config_by_name_missing_ok _null_ _null_ _null_ ));
+DESCR("SHOW X as a function; if second param is true then don't thrown an error if unset");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 0 f f f f f f v 3 0 25 "25 25 16" _null_ _null_ _null_ _null_ set_config_by_name _null_ _null_ _null_ ));
DESCR("SET X as a function");
DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t s 0 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,1009,25,25,25,23}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,boot_val,reset_val,sourcefile,sourceline}" _null_ show_all_settings _null_ _null_ _null_ ));
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 6310641..4d672ae 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -1095,6 +1095,7 @@ extern Datum quote_nullable(PG_FUNCTION_ARGS);
/* guc.c */
extern Datum show_config_by_name(PG_FUNCTION_ARGS);
+extern Datum show_config_by_name_fallback(PG_FUNCTION_ARGS);
extern Datum set_config_by_name(PG_FUNCTION_ARGS);
extern Datum show_all_settings(PG_FUNCTION_ARGS);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index d3100d1..3f294f1 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -351,9 +351,11 @@ extern int set_config_option(const char *name, const char *value,
GucAction action, bool changeVal, int elevel,
bool is_reload);
extern void AlterSystemSetConfigFile(AlterSystemStmt *setstmt);
-extern char *GetConfigOptionByName(const char *name, const char **varname);
+
+extern char *GetConfigOptionByNameMissingOK(const char *name, const char **varname, bool missing_ok);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
+#define GetConfigOptionByName(name,var) GetConfigOptionByNameMissingOK(name,var,false);
extern void SetPGVariable(const char *name, List *args, bool is_local);
extern void GetPGVariable(const char *name, DestReceiver *dest);
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 4f0065c..e20f6a9 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -736,3 +736,39 @@ NOTICE: text search configuration "no_such_config" does not exist
select func_with_bad_set();
ERROR: invalid value for parameter "default_text_search_config": "no_such_config"
reset check_function_bodies;
+-- check multi-arg custom current_setting
+-- this should error:
+select current_setting('nosuch.setting');
+ERROR: unrecognized configuration parameter "nosuch.setting"
+-- this should also error
+select current_setting('nosuch.setting',false);
+ERROR: unrecognized configuration parameter "nosuch.setting"
+-- this should return NULL
+select current_setting('nosuch.setting',true) is null;
+ ?column?
+----------
+ t
+(1 row)
+
+-- this should now return 'nada'
+set nosuch.setting = 'nada';
+select current_setting('nosuch.setting');
+ current_setting
+-----------------
+ nada
+(1 row)
+
+-- as should this
+select current_setting('nosuch.setting',false);
+ current_setting
+-----------------
+ nada
+(1 row)
+
+-- as should this
+select current_setting('nosuch.setting',true);
+ current_setting
+-----------------
+ nada
+(1 row)
+
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 3de8a6b..a1561f2 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -275,3 +275,25 @@ set default_text_search_config = no_such_config;
select func_with_bad_set();
reset check_function_bodies;
+
+-- check multi-arg custom current_setting
+
+-- this should error:
+select current_setting('nosuch.setting');
+
+-- this should also error
+select current_setting('nosuch.setting',false);
+
+-- this should return NULL
+select current_setting('nosuch.setting',true) is null;
+
+-- this should now return 'nada'
+set nosuch.setting = 'nada';
+
+select current_setting('nosuch.setting');
+
+-- as should this
+select current_setting('nosuch.setting',false);
+
+-- as should this
+select current_setting('nosuch.setting',true);
--
2.3.3
Hello,
Well, speaking of the two-arg form vs alternate name, here's a version of
the patch which includes the >new behavior
Thought i will attempt a review.
The patch applies cleanly to latest HEAD.
patch -p1 <
/home/Sameer/Downloads/0001-Add-two-arg-form-of-current_setting-to-optionally-su.patch
patching file doc/src/sgml/func.sgml
Hunk #1 succeeded at 16216 (offset 1 line).
Hunk #2 succeeded at 16255 (offset 1 line).
patching file src/backend/utils/misc/guc.c
Hunk #1 succeeded at 7693 (offset -3 lines).
Hunk #2 succeeded at 8012 (offset -3 lines).
patching file src/include/catalog/pg_proc.h
Hunk #1 succeeded at 3044 (offset 4 lines).
patching file src/include/utils/builtins.h
patching file src/include/utils/guc.h
patching file src/test/regress/expected/guc.out
patching file src/test/regress/sql/guc.sql
But i do get error at make
make -C catalog schemapg.h
make[3]: Entering directory
`/home/Sameer/git/latest_postgres/postgres/src/backend/catalog'
cd ../../../src/include/catalog && '/usr/bin/perl' ./duplicate_oids
3280
make[3]: *** [postgres.bki] Error 1
make[3]: Leaving directory
`/home/Sameer/git/latest_postgres/postgres/src/backend/catalog'
make[2]: *** [submake-schemapg] Error 2
make[2]: Leaving directory
`/home/Sameer/git/latest_postgres/postgres/src/backend'
make[1]: *** [all-backend-recurse] Error 2
make[1]: Leaving directory `/home/Sameer/git/latest_postgres/postgres/src'
make: *** [all-src-recurse] Error 2
regards
Sameer
--
View this message in context: http://postgresql.nabble.com/PATCH-two-arg-current-setting-with-fallback-tp5842654p5847904.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed
I have reviewed the patch.
Here are my review comments:
1. Patch does not apply due to some recent changes in pg_proc.h
2.
-GetConfigOptionByName(const char *name, const char **varname)
+GetConfigOptionByNameMissingOK(const char *name, const char **varname, bool missing_ok)Will it be better if we keep the name as is and change the callers to pass
false for missing_ok parameter?
It looks weired to have an extra #define just to avoid that.
I see countable callers and thus see NO issues changing those.
3. Oid used for new function is already used. Check unused_oids.sh.
4. Changes in builtins.h are accidental. Need to remove that.
However, code changes looks good and implements the desired feature.
The new status of this patch is: Waiting on Author
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
Attached patch which fixes my review comments.
Since code changes were good, just fixed reported cosmetic changes.
David, can you please cross check?
Thanks
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Attachments:
0002-Add-two-arg-form-of-current_setting-Jeevan.patchtext/x-patch; charset=US-ASCII; name=0002-Add-two-arg-form-of-current_setting-Jeevan.patchDownload
diff --git a/contrib/tsearch2/tsearch2.c b/contrib/tsearch2/tsearch2.c
index 143dabb..4354c5b 100644
--- a/contrib/tsearch2/tsearch2.c
+++ b/contrib/tsearch2/tsearch2.c
@@ -363,7 +363,7 @@ tsa_tsearch2(PG_FUNCTION_ARGS)
tgargs[i + 1] = trigger->tgargs[i];
tgargs[1] = pstrdup(GetConfigOptionByName("default_text_search_config",
- NULL));
+ NULL, false));
tgargs_old = trigger->tgargs;
trigger->tgargs = tgargs;
trigger->tgnargs++;
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index c6e3540..7f6c4ad 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16435,7 +16435,7 @@ SELECT collation for ('foo' COLLATE "de_DE");
<indexterm>
<primary>current_setting</primary>
</indexterm>
- <literal><function>current_setting(<parameter>setting_name</parameter>)</function></literal>
+ <literal><function>current_setting(<parameter>setting_name</parameter> [, <parameter>missing_ok</parameter> ])</function></literal>
</entry>
<entry><type>text</type></entry>
<entry>get current value of setting</entry>
@@ -16474,7 +16474,9 @@ SELECT collation for ('foo' COLLATE "de_DE");
The function <function>current_setting</function> yields the
current value of the setting <parameter>setting_name</parameter>.
It corresponds to the <acronym>SQL</acronym> command
- <command>SHOW</command>. An example:
+ <command>SHOW</command>. If <parameter>setting</parameter> does not exist,
+ throws an error unless <parameter>missing_ok</parameter> is
+ <literal>true</literal>. An example:
<programlisting>
SELECT current_setting('datestyle');
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index b3c9f14..a43925d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7137,7 +7137,7 @@ ExtractSetVariableArgs(VariableSetStmt *stmt)
case VAR_SET_VALUE:
return flatten_set_variable_args(stmt->name, stmt->args);
case VAR_SET_CURRENT:
- return GetConfigOptionByName(stmt->name, NULL);
+ return GetConfigOptionByName(stmt->name, NULL, false);
default:
return NULL;
}
@@ -7206,7 +7206,7 @@ set_config_by_name(PG_FUNCTION_ARGS)
true, 0, false);
/* get the new current value */
- new_value = GetConfigOptionByName(name, NULL);
+ new_value = GetConfigOptionByName(name, NULL, false);
/* Convert return string to text */
PG_RETURN_TEXT_P(cstring_to_text(new_value));
@@ -7633,7 +7633,7 @@ GetPGVariableResultDesc(const char *name)
const char *varname;
/* Get the canonical spelling of name */
- (void) GetConfigOptionByName(name, &varname);
+ (void) GetConfigOptionByName(name, &varname, false);
/* need a tuple descriptor representing a single TEXT column */
tupdesc = CreateTemplateTupleDesc(1, false);
@@ -7656,7 +7656,7 @@ ShowGUCConfigOption(const char *name, DestReceiver *dest)
char *value;
/* Get the value and canonical spelling of name */
- value = GetConfigOptionByName(name, &varname);
+ value = GetConfigOptionByName(name, &varname, false);
/* need a tuple descriptor representing a single TEXT column */
tupdesc = CreateTemplateTupleDesc(1, false);
@@ -7740,19 +7740,26 @@ ShowAllGUCConfig(DestReceiver *dest)
}
/*
- * Return GUC variable value by name; optionally return canonical
- * form of name. Return value is palloc'd.
+ * Return GUC variable value by name; optionally return canonical form of
+ * name. If the GUC is unset, then throw an error unless missing_ok is true,
+ * in which case return NULL. Return value is palloc'd.
*/
char *
-GetConfigOptionByName(const char *name, const char **varname)
+GetConfigOptionByName(const char *name, const char **varname, bool missing_ok)
{
struct config_generic *record;
record = find_option(name, false, ERROR);
if (record == NULL)
+ {
+ if (missing_ok)
+ return NULL;
+
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("unrecognized configuration parameter \"%s\"", name)));
+ }
+
if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -8046,12 +8053,42 @@ show_config_by_name(PG_FUNCTION_ARGS)
varname = TextDatumGetCString(PG_GETARG_DATUM(0));
/* Get the value */
- varval = GetConfigOptionByName(varname, NULL);
+ varval = GetConfigOptionByName(varname, NULL, false);
+
+ /* Convert to text */
+ PG_RETURN_TEXT_P(cstring_to_text(varval));
+}
+
+/*
+ * show_config_by_name_missing_ok - equiv to SHOW X command but implemented as
+ * a function. If X does not exist, suppress the error and just return NULL
+ * if missing_ok is TRUE.
+ */
+Datum
+show_config_by_name_missing_ok(PG_FUNCTION_ARGS)
+{
+ char *varname;
+ bool missing_ok;
+ char *varval;
+
+ /* Get the GUC variable name */
+ varname = TextDatumGetCString(PG_GETARG_DATUM(0));
+
+ /* Get the missing_ok value */
+ missing_ok = PG_GETARG_BOOL(1);
+
+ /* Get the value */
+ varval = GetConfigOptionByName(varname, NULL, missing_ok);
+
+ /* return NULL if this was NULL */
+ if (!varval)
+ PG_RETURN_NULL();
/* Convert to text */
PG_RETURN_TEXT_P(cstring_to_text(varval));
}
+
/*
* show_all_settings - equiv to SHOW ALL command but implemented as
* a Table Function.
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index fe06ec2..6468be4 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3067,6 +3067,8 @@ DESCR("convert bitstring to int8");
DATA(insert OID = 2077 ( current_setting PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 25 "25" _null_ _null_ _null_ _null_ _null_ show_config_by_name _null_ _null_ _null_ ));
DESCR("SHOW X as a function");
+DATA(insert OID = 4066 ( current_setting PGNSP PGUID 12 1 0 0 0 f f f f t f s 2 0 25 "25 16" _null_ _null_ _null_ _null_ _null_ show_config_by_name_missing_ok _null_ _null_ _null_ ));
+DESCR("SHOW X as a function; if second argument is true then don't thrown an error if unset");
DATA(insert OID = 2078 ( set_config PGNSP PGUID 12 1 0 0 0 f f f f f f v 3 0 25 "25 25 16" _null_ _null_ _null_ _null_ _null_ set_config_by_name _null_ _null_ _null_ ));
DESCR("SET X as a function");
DATA(insert OID = 2084 ( pg_show_all_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t s 0 0 2249 "" "{25,25,25,25,25,25,25,25,25,25,25,1009,25,25,25,23,16}" "{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}" "{name,setting,unit,category,short_desc,extra_desc,context,vartype,source,min_val,max_val,enumvals,boot_val,reset_val,sourcefile,sourceline,pending_restart}" _null_ _null_ show_all_settings _null_ _null_ _null_ ));
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index ffe1168..ed62ee4 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -355,7 +355,9 @@ extern int set_config_option(const char *name, const char *value,
GucAction action, bool changeVal, int elevel,
bool is_reload);
extern void AlterSystemSetConfigFile(AlterSystemStmt *setstmt);
-extern char *GetConfigOptionByName(const char *name, const char **varname);
+
+extern char *GetConfigOptionByName(const char *name, const char **varname,
+ bool missing_ok);
extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
extern int GetNumConfigOptions(void);
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 4f0065c..e20f6a9 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -736,3 +736,39 @@ NOTICE: text search configuration "no_such_config" does not exist
select func_with_bad_set();
ERROR: invalid value for parameter "default_text_search_config": "no_such_config"
reset check_function_bodies;
+-- check multi-arg custom current_setting
+-- this should error:
+select current_setting('nosuch.setting');
+ERROR: unrecognized configuration parameter "nosuch.setting"
+-- this should also error
+select current_setting('nosuch.setting',false);
+ERROR: unrecognized configuration parameter "nosuch.setting"
+-- this should return NULL
+select current_setting('nosuch.setting',true) is null;
+ ?column?
+----------
+ t
+(1 row)
+
+-- this should now return 'nada'
+set nosuch.setting = 'nada';
+select current_setting('nosuch.setting');
+ current_setting
+-----------------
+ nada
+(1 row)
+
+-- as should this
+select current_setting('nosuch.setting',false);
+ current_setting
+-----------------
+ nada
+(1 row)
+
+-- as should this
+select current_setting('nosuch.setting',true);
+ current_setting
+-----------------
+ nada
+(1 row)
+
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 3de8a6b..a1561f2 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -275,3 +275,25 @@ set default_text_search_config = no_such_config;
select func_with_bad_set();
reset check_function_bodies;
+
+-- check multi-arg custom current_setting
+
+-- this should error:
+select current_setting('nosuch.setting');
+
+-- this should also error
+select current_setting('nosuch.setting',false);
+
+-- this should return NULL
+select current_setting('nosuch.setting',true) is null;
+
+-- this should now return 'nada'
+set nosuch.setting = 'nada';
+
+select current_setting('nosuch.setting');
+
+-- as should this
+select current_setting('nosuch.setting',false);
+
+-- as should this
+select current_setting('nosuch.setting',true);
Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
Attached patch which fixes my review comments.
Applied with minor adjustments (mostly cosmetic, but did neither of you
notice the compiler warning?)
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jul 3, 2015 at 2:12 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
Attached patch which fixes my review comments.
Applied with minor adjustments (mostly cosmetic, but did neither of you
notice the compiler warning?)
Oops. Sorry for that.
Added -Wall -Werror in my configuration.
Thanks
regards, tom lane
--
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company