Re: Improvements in psql hooks for variables - Mailing list pgsql-hackers
| From | Rahila Syed |
|---|---|
| Subject | Re: Improvements in psql hooks for variables |
| Date | |
| Msg-id | CAH2L28vGyL8Wiv1OKB+OzCd1q=WZh41mcpGzu-2-jUmXOCgYJg@mail.gmail.com Whole thread Raw |
| In response to | Re: Improvements in psql hooks for variables (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>) |
| Responses |
Re: Improvements in psql hooks for variables
|
| List | 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);
^
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 by date: