From f3f06b13308e3c250bb8b0afee861744cda06cf6 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 9 Oct 2024 18:47:34 +0900 Subject: [PATCH] Apply GUC name from central table in more places of guc.c The name extracted from the record of the GUC tables is applied to more internal places of guc.c. This change has the advantage to simplify parse_and_validate_value(), where the "name" was only used in elog messages, while it was required to match with the name from the GUC record. pg_parameter_aclcheck() now passes the name of the GUC from its record in two places rather than the caller's argument. The value given to this function goes through convert_GUC_name_for_parameter_acl() that does a simple ASCII downcasing. Few GUCs mix character casing in core; one test is added for one of these code paths with "IntervalStyle". Author: Peter Smith, Michael Paquier Discussion: https://postgr.es/m/ZwNh4vkc2NHJHnND@paquier.xyz --- src/backend/utils/misc/guc.c | 65 +++++++++++++++++++-------------------- src/test/regress/expected/guc.out | 4 +++ src/test/regress/sql/guc.sql | 3 ++ 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 13527fc258..507a5d329a 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -3115,7 +3115,6 @@ config_enum_get_options(struct config_enum *record, const char *prefix, * and also calls any check hook the parameter may have. * * record: GUC variable's info record - * name: variable name (should match the record of course) * value: proposed value, as a string * source: identifies source of value (check hooks may need this) * elevel: level to log any error reports at @@ -3127,7 +3126,7 @@ config_enum_get_options(struct config_enum *record, const char *prefix, */ static bool parse_and_validate_value(struct config_generic *record, - const char *name, const char *value, + const char *value, GucSource source, int elevel, union config_var_val *newval, void **newextra) { @@ -3142,7 +3141,7 @@ parse_and_validate_value(struct config_generic *record, ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("parameter \"%s\" requires a Boolean value", - name))); + conf->gen.name))); return false; } @@ -3162,7 +3161,7 @@ parse_and_validate_value(struct config_generic *record, ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid value for parameter \"%s\": \"%s\"", - name, value), + conf->gen.name, value), hintmsg ? errhint("%s", _(hintmsg)) : 0)); return false; } @@ -3181,7 +3180,7 @@ parse_and_validate_value(struct config_generic *record, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("%d%s%s is outside the valid range for parameter \"%s\" (%d%s%s .. %d%s%s)", newval->intval, unitspace, unit, - name, + conf->gen.name, conf->min, unitspace, unit, conf->max, unitspace, unit))); return false; @@ -3203,7 +3202,7 @@ parse_and_validate_value(struct config_generic *record, ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid value for parameter \"%s\": \"%s\"", - name, value), + conf->gen.name, value), hintmsg ? errhint("%s", _(hintmsg)) : 0)); return false; } @@ -3222,7 +3221,7 @@ parse_and_validate_value(struct config_generic *record, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("%g%s%s is outside the valid range for parameter \"%s\" (%g%s%s .. %g%s%s)", newval->realval, unitspace, unit, - name, + conf->gen.name, conf->min, unitspace, unit, conf->max, unitspace, unit))); return false; @@ -3278,7 +3277,7 @@ parse_and_validate_value(struct config_generic *record, ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid value for parameter \"%s\": \"%s\"", - name, value), + conf->gen.name, value), hintmsg ? errhint("%s", _(hintmsg)) : 0)); if (hintmsg) @@ -3460,7 +3459,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_INVALID_TRANSACTION_STATE), errmsg("parameter \"%s\" cannot be set during a parallel operation", - name))); + record->name))); return 0; } @@ -3476,7 +3475,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed", - name))); + record->name))); return 0; } break; @@ -3499,7 +3498,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", - name))); + record->name))); return 0; } break; @@ -3509,7 +3508,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed now", - name))); + record->name))); return 0; } @@ -3529,14 +3528,14 @@ set_config_with_handle(const char *name, config_handle *handle, */ AclResult aclresult; - aclresult = pg_parameter_aclcheck(name, srole, ACL_SET); + aclresult = pg_parameter_aclcheck(record->name, srole, ACL_SET); if (aclresult != ACLCHECK_OK) { /* No granted privilege */ ereport(elevel, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to set parameter \"%s\"", - name))); + record->name))); return 0; } } @@ -3578,7 +3577,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be set after connection start", - name))); + record->name))); return 0; } break; @@ -3591,14 +3590,14 @@ set_config_with_handle(const char *name, config_handle *handle, */ AclResult aclresult; - aclresult = pg_parameter_aclcheck(name, srole, ACL_SET); + aclresult = pg_parameter_aclcheck(record->name, srole, ACL_SET); if (aclresult != ACLCHECK_OK) { /* No granted privilege */ ereport(elevel, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied to set parameter \"%s\"", - name))); + record->name))); return 0; } } @@ -3637,7 +3636,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("cannot set parameter \"%s\" within security-definer function", - name))); + record->name))); return 0; } if (InSecurityRestrictedOperation()) @@ -3645,7 +3644,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("cannot set parameter \"%s\" within security-restricted operation", - name))); + record->name))); return 0; } } @@ -3657,7 +3656,7 @@ set_config_with_handle(const char *name, config_handle *handle, { ereport(elevel, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("parameter \"%s\" cannot be reset", name))); + errmsg("parameter \"%s\" cannot be reset", record->name))); return 0; } if (action == GUC_ACTION_SAVE) @@ -3665,7 +3664,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("parameter \"%s\" cannot be set locally in functions", - name))); + record->name))); return 0; } } @@ -3691,7 +3690,7 @@ set_config_with_handle(const char *name, config_handle *handle, if (changeVal && !makeDefault) { elog(DEBUG3, "\"%s\": setting ignored because previous source is higher priority", - name); + record->name); return -1; } changeVal = false; @@ -3710,7 +3709,7 @@ set_config_with_handle(const char *name, config_handle *handle, if (value) { - if (!parse_and_validate_value(record, name, value, + if (!parse_and_validate_value(record, value, source, elevel, &newval_union, &newextra)) return 0; @@ -3743,7 +3742,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", - name))); + conf->gen.name))); return 0; } record->status &= ~GUC_PENDING_RESTART; @@ -3808,7 +3807,7 @@ set_config_with_handle(const char *name, config_handle *handle, if (value) { - if (!parse_and_validate_value(record, name, value, + if (!parse_and_validate_value(record, value, source, elevel, &newval_union, &newextra)) return 0; @@ -3841,7 +3840,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", - name))); + conf->gen.name))); return 0; } record->status &= ~GUC_PENDING_RESTART; @@ -3906,7 +3905,7 @@ set_config_with_handle(const char *name, config_handle *handle, if (value) { - if (!parse_and_validate_value(record, name, value, + if (!parse_and_validate_value(record, value, source, elevel, &newval_union, &newextra)) return 0; @@ -3939,7 +3938,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", - name))); + conf->gen.name))); return 0; } record->status &= ~GUC_PENDING_RESTART; @@ -4004,7 +4003,7 @@ set_config_with_handle(const char *name, config_handle *handle, if (value) { - if (!parse_and_validate_value(record, name, value, + if (!parse_and_validate_value(record, value, source, elevel, &newval_union, &newextra)) return 0; @@ -4063,7 +4062,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", - name))); + conf->gen.name))); return 0; } record->status &= ~GUC_PENDING_RESTART; @@ -4133,7 +4132,7 @@ set_config_with_handle(const char *name, config_handle *handle, if (value) { - if (!parse_and_validate_value(record, name, value, + if (!parse_and_validate_value(record, value, source, elevel, &newval_union, &newextra)) return 0; @@ -4166,7 +4165,7 @@ set_config_with_handle(const char *name, config_handle *handle, ereport(elevel, (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM), errmsg("parameter \"%s\" cannot be changed without restarting the server", - name))); + conf->gen.name))); return 0; } record->status &= ~GUC_PENDING_RESTART; @@ -4660,7 +4659,7 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) union config_var_val newval; void *newextra = NULL; - if (!parse_and_validate_value(record, name, value, + if (!parse_and_validate_value(record, value, PGC_S_FILE, ERROR, &newval, &newextra)) ereport(ERROR, diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 14edb42704..7f9e29c765 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -6,6 +6,10 @@ SHOW datestyle; Postgres, MDY (1 row) +-- Check output style of CamelCase enum options +SET intervalstyle to 'asd'; +ERROR: invalid value for parameter "IntervalStyle": "asd" +HINT: Available values: postgres, postgres_verbose, sql_standard, iso_8601. -- SET to some nondefault value SET vacuum_cost_delay TO 40; SET datestyle = 'ISO, YMD'; diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index 2be7ab2940..f65f84a263 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -2,6 +2,9 @@ -- we can't rely on any specific default value of vacuum_cost_delay SHOW datestyle; +-- Check output style of CamelCase enum options +SET intervalstyle to 'asd'; + -- SET to some nondefault value SET vacuum_cost_delay TO 40; SET datestyle = 'ISO, YMD'; -- 2.11.4.GIT