Rahila Syed wrote: > However, including the check here will require returning the status > of command from this hook. i.e if we throw error inside this > hook we will need to return false indicating the value has not changed.
Looking at the other variables hooks, they already emit errors and can deny the effect of a change corresponding to a new value, without informing the caller. Why would autocommit be different? For example echo_hook does this: /* ...if the value is in (queries,errors,all,none) then assign pset.echo accordingly ... */ else { psql_error("unrecognized value \"%s\" for \"%s\"; assuming \"%s\"\n", newval, "ECHO", "none"); pset.echo = PSQL_ECHO_NONE; } If the user issues \set ECHO FOOBAR, then it produces the above error message and makes the same internal change as if \set ECHO none had been issued. But, the value of the variable as seen by the user is still FOOBAR: \set [...other variables...] ECHO = 'FOOBAR' The proposed patch doesn't follow that behavior, as it denies a new value for AUTOCOMMIT. You might argue that it's better, but on the other side, there are two reasons against it: - it's not consistent with the other uses of \set that affect psql, such as ECHO, ECHO_HIDDEN, ON_ERROR_ROLLBACK, COMP_KEYWORD_CASE... and even AUTOCOMMIT as \set AUTOCOMMIT FOOBAR is not denied, just reinterpreted. - it doesn't address the case of another method than \set being used to set the variable, as in : SELECT 'on' as "AUTOCOMMIT" \gset whereas the hook would work in that case. 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