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.ba...@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 <dan...@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 >