Re: Improvements in psql hooks for variables

From: "Daniel Verite" <daniel(at)manitou-mail(dot)org>
To: "Stephen Frost" <sfrost(at)snowman(dot)net>
Cc: "Rahila Syed" <rahilasyed90(at)gmail(dot)com>,"Ashutosh Bapat" <ashutosh(dot)bapat(at)enterprisedb(dot)com>,"pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improvements in psql hooks for variables
Date: 2016-11-21 13:44:37
Message-ID: 14360f32-7d7e-4452-8fd5-e726477272b1@mm
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Stephen Frost wrote:

> Just fyi, there seems to be some issues with this patch because setting
> my PROMPT1 and PROMPT2 variables result in rather odd behavior.

Good catch! The issue is that the patch broke the assumption
of prompt hooks that their argument points to a long-lived buffer.
The attached v4 fixes the bug by passing to hooks a pointer to the final
strdup'ed value in VariableSpace rather than temp space, as commented
in SetVariable().

Also I've changed something else in ParseVariableBool(). The code assumes
"false" when value==NULL, but when value is an empty string, the result
was true and considered valid, due to the following test being
positive when len==0 (both with HEAD or the v3 patch from this thread):

if (pg_strncasecmp(value, "true", len) == 0)
return true;
It happens that "" as a value yields the same result than "true" for this
test so it passes, but it seems rather unintentional.

The resulting problem from the POV of the user is that we can do that,
for instance:

test=> \set AUTOCOMMIT

without error message or feedback, and that leaves us without much
clue about autocommit being enabled:

test=> \echo :AUTOCOMMIT

test=>

So I've changed ParseVariableBool() in v4 to reject empty string as an
invalid boolean (but not NULL since the startup logic requires NULL
meaning false in the early initialization of these variables).

"make check" seems OK with that, I hope it doesn't cause any regression
elsewhere.

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

Attachment Content-Type Size
psql-var-hooks-v4.patch text/plain 13.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Verite 2016-11-21 13:49:45 Re: Improvements in psql hooks for variables
Previous Message Stephen Frost 2016-11-21 13:41:35 Re: Improvements in psql hooks for variables