Re: Improvements in psql hooks for variables

Поиск
Список
Период
Сортировка
От Rahila Syed
Тема Re: Improvements in psql hooks for variables
Дата
Msg-id CAH2L28vGyL8Wiv1OKB+OzCd1q=WZh41mcpGzu-2-jUmXOCgYJg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Improvements in psql hooks for variables  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Ответы Re: Improvements in psql hooks for variables  ("Daniel Verite" <daniel@manitou-mail.org>)
Список pgsql-hackers
Hello,

I am beginning to review this patch. Initial comment. I got following compilation error when I applied the patch on latest sources and run make.

command.c: In function ‘exec_command’:
command.c:257:5: error: too few arguments to function ‘ParseVariableBool’
     ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix) ?
     ^
In file included from settings.h:12:0,
                 from command.c:50:
variables.h:38:7: note: declared here
 bool  ParseVariableBool(const char *value, const char *name, bool *valid);
       ^
command.c:1551:4: error: too few arguments to function ‘ParseVariableBool’
    pset.timing = ParseVariableBool(opt, "\\timing");
    ^
In file included from settings.h:12:0,
                 from command.c:50:
variables.h:38:7: note: declared here
 bool  ParseVariableBool(const char *value, const char *name, bool *valid);
       ^
command.c: In function ‘do_pset’:
command.c:2663:4: error: too few arguments to function ‘ParseVariableBool’
    popt->topt.expanded = ParseVariableBool(value, param);
    ^
In file included from settings.h:12:0,
                 from command.c:50:
variables.h:38:7: note: declared here
 bool  ParseVariableBool(const char *value, const char *name, bool *valid);
       ^
command.c:2672:4: error: too few arguments to function ‘ParseVariableBool’
    popt->topt.numericLocale = ParseVariableBool(value, param);
    ^
In file included from settings.h:12:0,
                 from command.c:50:
variables.h:38:7: note: declared here
 bool  ParseVariableBool(const char *value, const char *name, bool *valid);
       ^
command.c:2727:4: error: too few arguments to function ‘ParseVariableBool’
    popt->topt.tuples_only = ParseVariableBool(value, param);
    ^
In file included from settings.h:12:0,
                 from command.c:50:
variables.h:38:7: note: declared here
 bool  ParseVariableBool(const char *value, const char *name, bool *valid);
       ^
command.c:2759:4: error: too few arguments to function ‘ParseVariableBool’
    if (ParseVariableBool(value, param))
    ^
In file included from settings.h:12:0,
                 from command.c:50:
variables.h:38:7: note: declared here
 bool  ParseVariableBool(const char *value, const char *name, bool *valid);
       ^
command.c:2781:4: error: too few arguments to function ‘ParseVariableBool’
    popt->topt.default_footer = ParseVariableBool(value, param);
    ^
In file included from settings.h:12:0,
                 from command.c:50:
variables.h:38:7: note: declared here
 bool  ParseVariableBool(const char *value, const char *name, bool *valid);
       ^

Thank you,
Rahila Syed


On Mon, Sep 19, 2016 at 11:21 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:
You may want to add this to the November commitfest
https://commitfest.postgresql.org/11/.

On Fri, Sep 16, 2016 at 6:05 PM, Daniel Verite <daniel@manitou-mail.org> wrote:
>  Hi,
>
> Following the discussion on forbidding an AUTOCOMMIT off->on
> switch mid-transaction [1], attached is a patch that let the hooks
> return a boolean indicating whether a change is allowed.
>
> Using the hooks, bogus assignments to built-in variables can
> be dealt with more strictly.
>
> For example, pre-patch behavior:
>
>   =# \set ECHO errors
>   =# \set ECHO on
>   unrecognized value "on" for "ECHO"; assuming "none"
>   =# \echo :ECHO
>   on
>
> which has two problems:
> - we have to assume a value, even though we can't know what the user meant.
> - after assignment, the user-visible value of the variable diverges from its
> internal counterpart (pset.echo in this case).
>
>
> Post-patch:
>   =# \set ECHO errors
>   =# \set ECHO on
>   unrecognized value "on" for "ECHO"
>   \set: error while setting variable
>   =# \echo :ECHO
>   errors
>
> Both the internal pset.* state and the user-visible value are kept unchanged
> is the input value is incorrect.
>
> Concerning AUTOCOMMIT, autocommit_hook() could return false to forbid
> a switch when the conditions are not met.
>
>
> Another user-visible effect of the patch is that, using a bogus value
> for a built-in variable on the command-line becomes a fatal error
> that prevents psql to continue.
> This is not directly intended by the patch but is a consequence
> of SetVariable() failing.
>
> Example:
>   $ ./psql -vECHO=bogus
>   unrecognized value "bogus" for "ECHO"
>   psql: could not set variable "ECHO"
>   $ echo $?
>   1
>
> The built-in vars concerned by the change are:
>
> booleans: AUTOCOMMIT, ON_ERROR_STOP, QUIET, SINGLELINE, SINGLESTEP
>
> non-booleans: ECHO, ECHO_HIDDEN, ON_ERROR_ROLLBACK, COMP_KEYWORD_CASE,
>  HISTCONTROL, VERBOSITY, SHOW_CONTEXT
>
> We could go further to close the gap between pset.* and the built-in
> variables,
> by changing how they're initialized and forbidding deletion as Tom
> suggests in [2], but if there's negative feedback on the above changes,
> I think we should hear it first.
>
> [1]
> https://www.postgresql.org/message-id/f2cb5838-0ee9-4fe3-acc0-df77aeb7d4c7%40mm
> [2]
> https://www.postgresql.org/message-id/4695.1473961140%40sss.pgh.pa.us
>
>
> Best regards,
> --
> Daniel Vérité
> PostgreSQL-powered mailer: http://www.manitou-mail.org
> Twitter: @DanielVerite
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Amit Kapila
Дата:
Сообщение: Re: Hash Indexes
Следующее
От: Antonin Houska
Дата:
Сообщение: Possibly too stringent Assert() in b-tree code