Publish GUC flags to custom variables
Hello,
Postgres supports to add custom GUC variables on runtime, but we
cannot use GUC flags in them. This patch adds the flags argument
to DefineCusomXxx() functions. The flags were always 0 until now.
GUC flags are useful for variables with units. Users will be able
to add configuration parameters somothing like memory-size or
time-duration more easily.
I have a plan to use the feature in SQL tracing and analyzing add-on
for postgres. Also, the auto-explain patch suggested in the last
commit-fest could be re-implemented as a plug-in instead of a core-feature
using the custom variable with units and ExecutorRun_hook.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
custom_guc_flags.patchapplication/octet-stream; name=custom_guc_flags.patchDownload
diff -cpr HEAD/src/backend/utils/misc/guc.c custom_guc_flags/src/backend/utils/misc/guc.c
*** HEAD/src/backend/utils/misc/guc.c Fri Aug 15 17:37:40 2008
--- custom_guc_flags/src/backend/utils/misc/guc.c Tue Aug 19 13:28:27 2008
*************** static struct config_generic *
*** 5586,5591 ****
--- 5586,5592 ----
init_custom_variable(const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
GucContext context,
enum config_type type,
size_t sz)
*************** init_custom_variable(const char *name,
*** 5600,5605 ****
--- 5601,5607 ----
gen->group = CUSTOM_OPTIONS;
gen->short_desc = short_desc;
gen->long_desc = long_desc;
+ gen->flags = flags;
gen->vartype = type;
return gen;
*************** void
*** 5675,5680 ****
--- 5677,5683 ----
DefineCustomBoolVariable(const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
bool *valueAddr,
GucContext context,
GucBoolAssignHook assign_hook,
*************** DefineCustomBoolVariable(const char *nam
*** 5683,5689 ****
struct config_bool *var;
var = (struct config_bool *)
! init_custom_variable(name, short_desc, long_desc, context,
PGC_BOOL, sizeof(struct config_bool));
var->variable = valueAddr;
var->boot_val = *valueAddr;
--- 5686,5692 ----
struct config_bool *var;
var = (struct config_bool *)
! init_custom_variable(name, short_desc, long_desc, flags, context,
PGC_BOOL, sizeof(struct config_bool));
var->variable = valueAddr;
var->boot_val = *valueAddr;
*************** void
*** 5697,5702 ****
--- 5700,5706 ----
DefineCustomIntVariable(const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
int *valueAddr,
int minValue,
int maxValue,
*************** DefineCustomIntVariable(const char *name
*** 5707,5713 ****
struct config_int *var;
var = (struct config_int *)
! init_custom_variable(name, short_desc, long_desc, context,
PGC_INT, sizeof(struct config_int));
var->variable = valueAddr;
var->boot_val = *valueAddr;
--- 5711,5717 ----
struct config_int *var;
var = (struct config_int *)
! init_custom_variable(name, short_desc, long_desc, flags, context,
PGC_INT, sizeof(struct config_int));
var->variable = valueAddr;
var->boot_val = *valueAddr;
*************** void
*** 5723,5728 ****
--- 5727,5733 ----
DefineCustomRealVariable(const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
double *valueAddr,
double minValue,
double maxValue,
*************** DefineCustomRealVariable(const char *nam
*** 5733,5739 ****
struct config_real *var;
var = (struct config_real *)
! init_custom_variable(name, short_desc, long_desc, context,
PGC_REAL, sizeof(struct config_real));
var->variable = valueAddr;
var->boot_val = *valueAddr;
--- 5738,5744 ----
struct config_real *var;
var = (struct config_real *)
! init_custom_variable(name, short_desc, long_desc, flags, context,
PGC_REAL, sizeof(struct config_real));
var->variable = valueAddr;
var->boot_val = *valueAddr;
*************** void
*** 5749,5754 ****
--- 5754,5760 ----
DefineCustomStringVariable(const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
char **valueAddr,
GucContext context,
GucStringAssignHook assign_hook,
*************** DefineCustomStringVariable(const char *n
*** 5757,5763 ****
struct config_string *var;
var = (struct config_string *)
! init_custom_variable(name, short_desc, long_desc, context,
PGC_STRING, sizeof(struct config_string));
var->variable = valueAddr;
var->boot_val = *valueAddr;
--- 5763,5769 ----
struct config_string *var;
var = (struct config_string *)
! init_custom_variable(name, short_desc, long_desc, flags, context,
PGC_STRING, sizeof(struct config_string));
var->variable = valueAddr;
var->boot_val = *valueAddr;
*************** void
*** 5773,5778 ****
--- 5779,5785 ----
DefineCustomEnumVariable(const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
int *valueAddr,
const struct config_enum_entry *options,
GucContext context,
*************** DefineCustomEnumVariable(const char *nam
*** 5782,5788 ****
struct config_enum *var;
var = (struct config_enum *)
! init_custom_variable(name, short_desc, long_desc, context,
PGC_ENUM, sizeof(struct config_enum));
var->variable = valueAddr;
var->boot_val = *valueAddr;
--- 5789,5795 ----
struct config_enum *var;
var = (struct config_enum *)
! init_custom_variable(name, short_desc, long_desc, flags, context,
PGC_ENUM, sizeof(struct config_enum));
var->variable = valueAddr;
var->boot_val = *valueAddr;
diff -cpr HEAD/src/include/utils/guc.h custom_guc_flags/src/include/utils/guc.h
*** HEAD/src/include/utils/guc.h Thu Jul 24 02:29:53 2008
--- custom_guc_flags/src/include/utils/guc.h Tue Aug 19 13:28:27 2008
*************** extern void DefineCustomBoolVariable(
*** 163,168 ****
--- 163,169 ----
const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
bool *valueAddr,
GucContext context,
GucBoolAssignHook assign_hook,
*************** extern void DefineCustomIntVariable(
*** 172,177 ****
--- 173,179 ----
const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
int *valueAddr,
int minValue,
int maxValue,
*************** extern void DefineCustomRealVariable(
*** 183,188 ****
--- 185,191 ----
const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
double *valueAddr,
double minValue,
double maxValue,
*************** extern void DefineCustomStringVariable(
*** 194,199 ****
--- 197,203 ----
const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
char **valueAddr,
GucContext context,
GucStringAssignHook assign_hook,
*************** extern void DefineCustomEnumVariable(
*** 203,208 ****
--- 207,213 ----
const char *name,
const char *short_desc,
const char *long_desc,
+ int flags,
int *valueAddr,
const struct config_enum_entry *options,
GucContext context,
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
Postgres supports to add custom GUC variables on runtime, but we
cannot use GUC flags in them. This patch adds the flags argument
to DefineCusomXxx() functions. The flags were always 0 until now.
Of course the problem with this is that it breaks every external module
that is using the DefineCustom*Variable functions. I grant that we
often change backend APIs in ways that break external modules, but it's
a bit annoying to change an API that has no other purpose than to be
called by external modules. Is the functionality to be gained worth
that? Alternatively, should we uglify the patch by providing some form
of backwards compatibility --- ie, invent new names for the new forms of
the functions, and keep the old ones as-is? (I'm not really for that,
but the question needs to be asked.)
If we are going to change the API, I think we should take the
opportunity to clean up another problem, which is the nonintuitive
approach to establishing the new variable's boot_val default. ISTM
that should be specified explicitly as an additional argument to
DefineCustom*Variable. The current approach is that whatever is
physically in the variable at the time of the call determines the
default; this has confused people in the past
http://archives.postgresql.org/pgsql-hackers/2006-11/msg00925.php
and it's totally unlike the way things work for built-in GUC variables.
Another API question: should we allow the new variable's group to be set
to something other than CUSTOM_OPTIONS? I'm not sure that that's a good
idea, since the config_group values are ad-hoc and subject to change.
But again, now's the time to consider it.
regards, tom lane