Re: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)

Lists: pgsql-hackers
From: David Christensen <david(at)endpoint(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: David Christensen <david(at)endpoint(dot)com>
Subject: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)
Date: 2015-03-19 22:41:02
Message-ID: 1426804862-5899-1-git-send-email-david@endpoint.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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


From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: David Christensen <david(at)endpoint(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)
Date: 2017-11-01 17:27:22
Message-ID: CAFj8pRBjt-wyqf4aSAz-0Uk2jaGQ14T3K2GvRe-upD6rhtzOjg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi

better to send it as attachment

Regards

Pavel

2015-03-19 23:41 GMT+01:00 David Christensen <david(at)endpoint(dot)com>:

> 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
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: David Christensen <david(at)endpoint(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)
Date: 2017-11-01 17:51:48
Message-ID: CAKFQuwazKj9bTbc+viqK_xeCs8a_=_54JHabAG14UAYZvWLjCA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Mar 19, 2015 at 3:41 PM, David Christensen <david(at)endpoint(dot)com>
wrote:

> 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.:
>
> -- returns current setting of foo.bar, or 'default' if not set
> SELECT current_setting('foo.bar', 'default')
>

​​This doesn't actually change the GUC in the system, right? Do we want a
side-effect version of this?

There is already a two-arg form where the second argument is a boolean -
there needs to be tests that ensure proper behavior and function lookup
resolution.

No docs.

David J.


From: Nico Williams <nico(at)cryptonector(dot)com>
To: David Christensen <david(at)endpoint(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)
Date: 2017-11-01 18:02:38
Message-ID: 20171101180237.GO4496@localhost
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Mar 19, 2015 at 05:41:02PM -0500, David Christensen wrote:
> 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.:

There already _is_ a two-argument form of current_setting() that yours
somewhat conflicts with:

current_setting(setting_name [, missing_ok ])

https://www.postgresql.org/docs/current/static/functions-admin.html

I often use

coalesce(current_setting(setting_name, true), default_value_here)

as an implementation of current_setting() with a default value.

You could treat booleans as the second argument as a missing_ok argument
instead of a default value, since _currently_ current_setting() only
returns TEXT.

But if ever GUCs are allowed to have values of other types, then your
two-argument current_setting() will conflict with boolean GUCs.

There are several ways to prevent such a future conflict. Here are
some:

- make a two-argument current_setting() for boolean GUCs treat the
second argument as a default value (since there are no such GUCs
today, this won't break anything)

- use a new function name

- use a three-argument current_setting()

The third option seems very lame to me. So I'd argue for either of the
other two.

Nico
--


From: David Christensen <david(at)endpoint(dot)com>
To: Nico Williams <nico(at)cryptonector(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)
Date: 2017-11-01 18:27:06
Message-ID: A68856AF-3332-45E4-B380-084FD54945DB@endpoint.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


> On Nov 1, 2017, at 1:02 PM, Nico Williams <nico(at)cryptonector(dot)com> wrote:
>
> On Thu, Mar 19, 2015 at 05:41:02PM -0500, David Christensen wrote:
>> 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.:
>
> There already _is_ a two-argument form of current_setting() that yours
> somewhat conflicts with:
>
> current_setting(setting_name [, missing_ok ])
>
> https://www.postgresql.org/docs/current/static/functions-admin.html

Apologies; I have no idea how this email got re-sent, but it is quite old and a mistake.

David
--
David Christensen
End Point Corporation
david(at)endpoint(dot)com
785-727-1171


From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Nico Williams <nico(at)cryptonector(dot)com>, David Christensen <david(at)endpoint(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Add two-arg for of current_setting(NAME, FALLBACK)
Date: 2017-11-02 13:12:12
Message-ID: 17496ef9-4e3e-4e5b-1514-5406f5a41da2@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/1/17 14:02, Nico Williams wrote:
> There already _is_ a two-argument form of current_setting() that yours
> somewhat conflicts with:
>
> current_setting(setting_name [, missing_ok ])
>
> https://www.postgresql.org/docs/current/static/functions-admin.html
>
> I often use
>
> coalesce(current_setting(setting_name, true), default_value_here)
>
> as an implementation of current_setting() with a default value.

That appears to address this use case then. Do we need the new proposed
variant still?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services