Re: [HACKERS] Improvements in psql hooks for variables
От | Rahila Syed |
---|---|
Тема | Re: [HACKERS] Improvements in psql hooks for variables |
Дата | |
Msg-id | CAH2L28ugrq+N_=i8f6RvWL0NPUhT84CAp8SrDg1RoSc0T7+qBg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] Improvements in psql hooks for variables (Rahila Syed <rahilasyed90@gmail.com>) |
Ответы |
Re: [HACKERS] Improvements in psql hooks for variables
|
Список | pgsql-hackers |
Hello,
Please consider following comments on the patch.
Please consider following comments on the patch.
In function ParseVariableNum,
> if (!val || !val[0])
> return false;
Check for 'val == NULL' as in above condition is done even in callers of ParseVariableNum().
There should be only one such check.
>+ psql_error("Invalid value \"%s\" for \"%s\"\nAn integer is expected\n",
Cant we have this error in ParseVariableNum() similar to ParseVariableBool() ?
>+ /*
>+ * Switching AUTOCOMMIT from OFF to ON is not allowed when inside a
>+ * transaction, because it cannot be effective until the current
>+ * transaction is ended.
>+ */
>+ if (autocommit && !pset.autocommit &&
>+ pset.db && PQtransactionStatus(pset.db) == PQTRANS_INTRANS)
>+ {
>+ psql_error("cannot set AUTOCOMMIT to %s when inside a transaction\n", newval);
>+ }
The above change in autocommit behaviour needs to be documented.
Thank you,
Rahila Syed
On Wed, Jan 25, 2017 at 5:48 PM, Rahila Syed <rahilasyed90@gmail.com> wrote:
Rahila SyedThank you,I will post further comments on patch soon.Hello,The patch works fine on applying on latest master branch and testing it for various variables as listed in PsqlSettings struct.On Wed, Jan 25, 2017 at 1:33 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:"Daniel Verite" <daniel@manitou-mail.org> writes:
> Here's an update with these changes:
I scanned this patch very quickly and think it addresses my previous
stylistic objections. Rahila is the reviewer of record though, so
I'll wait for his comments before going further.
regards, tom lane
В списке pgsql-hackers по дате отправления: