Re: Improvements in psql hooks for variables
От | Daniel Verite |
---|---|
Тема | Re: Improvements in psql hooks for variables |
Дата | |
Msg-id | b0c61324-8ad3-413e-bb88-5c71d6412bba@mm обсуждение исходный текст |
Ответ на | Re: Improvements in psql hooks for variables (Rahila Syed <rahilasyed90@gmail.com>) |
Ответы |
Re: Improvements in psql hooks for variables
(Stephen Frost <sfrost@snowman.net>)
|
Список | pgsql-hackers |
Hi, I'm attaching v3 of the patch with error reporting for FETCH_COUNT as mentioned upthread, and rebased on the most recent master. > I was suggesting change on the lines of extending generic_boolean_hook to > include enum(part in enum_hooks which calls ParseVariableBool) and > integer parsing as well. Well, generic_boolean_hook() is meant to change this, for instance: static void on_error_stop_hook(const char *newval) { pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP"); } into that: static bool on_error_stop_hook(const char *newval) { return generic_boolean_hook(newval, "ON_ERROR_STOP", &pset.on_error_stop); } with the goal that the assignment does not occur if "newval" is bogus. The change is really minimal. When we're dealing with enum-or-bool variables, such as for instance ECHO_HIDDEN, the patch replaces this: static void echo_hidden_hook(const char *newval) { if (newval == NULL) pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; else if (pg_strcasecmp(newval, "noexec") == 0) pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC; else if (ParseVariableBool(newval, "ECHO_HIDDEN")) pset.echo_hidden = PSQL_ECHO_HIDDEN_ON; else /* ParseVariableBool printed msg if needed */ pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; } with that: static bool echo_hidden_hook(const char *newval) { if (newval == NULL) pset.echo_hidden = PSQL_ECHO_HIDDEN_OFF; else if (pg_strcasecmp(newval, "noexec") == 0) pset.echo_hidden = PSQL_ECHO_HIDDEN_NOEXEC; else { bool isvalid; bool val = ParseVariableBool(newval, "ECHO_HIDDEN", &isvalid); if (!isvalid) return false; /* ParseVariableBool printed msg */ pset.echo_hidden = val ? PSQL_ECHO_HIDDEN_ON : PSQL_ECHO_HIDDEN_OFF; } return true; } The goal being again to reject a bogus assignment, as opposed to replacing it with any hardwired value. Code-wise, we can't call generic_boolean_hook() here because we need to assign a non-boolean specific value after having parsed the ON/OFF user-supplied string. More generally, it turns out that the majority of hooks are concerned by this patch, as they parse user-supplied values, but there are 4 distinct categories of variables: 1- purely ON/OFF vars: AUTOCOMMIT, ON_ERROR_STOP, QUIET, SINGLELINE, SINGLESTEP 2- ON/OFF mixed with enum values: ECHO_HIDDEN, ON_ERROR_ROLLBACK 3- purely enum values: COMP_KEYWORD_CASE, HISTCONTROL, ECHO, VERBOSITY, SHOW_CONTEXT 4- numeric values: FETCH_COUNT If you suggest that the patch should refactor the implementation of hooks for case #2, only two hooks are concerned and they consist of non-mergeable enum-specific code interleaved with generic code, so I don't foresee any gain in fusioning. I have the same opinion about merging any of #1, #2, #3, #4 together. But feel free to post code implementing your idea if you disagree, maybe I just don't figure out what you have in mind. For case #3, these hooks clearly follow a common pattern, but I also don't see any benefit in an opportunistic rewrite given the nature of the functions. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Amit KapilaДата:
Сообщение: Re: Remove the comment on the countereffectiveness of large shared_buffers on Windows