Postgres Pro
Downloads
Home   >   mailing lists

Re: [HACKERS] Improvements in psql hooks for variables - Mailing list pgsql-hackers

From Rahila Syed
Subject Re: [HACKERS] Improvements in psql hooks for variables
Date
Msg-id CAH2L28ugrq+N_=i8f6RvWL0NPUhT84CAp8SrDg1RoSc0T7+qBg@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Improvements in psql hooks for variables  (Rahila Syed <rahilasyed90@gmail.com>)
Responses Re: [HACKERS] Improvements in psql hooks for variables  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Tree view
Hello,

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:
Hello,

The patch works fine on applying on latest master branch and testing it for various variables as listed in PsqlSettings struct.
I will post further comments on patch soon.

Thank you,
Rahila Syed

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 by date:

Previous
From: Michael Paquier
Date:
Subject: Re: [HACKERS] Create a separate test file for exercising system views
Next
From: Michael Paquier
Date:
Subject: Re: [HACKERS] sequence data type