Re: psql setenv command

From: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
To: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql setenv command
Date: 2011-09-26 21:07:54
Message-ID: CAMkU=1wOe247vW=hPrXgRna-8UPWpJUVVK=Xx=bQDC-Lk6xWUw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 26, 2011 at 12:07 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com> wrote:
> On Thu, Sep 15, 2011 at 7:02 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> On Thu, September 15, 2011 6:10 pm, Josh Kupershmidt wrote:
>>> [need way to show current values]
>> \! echo $foo
>>
>> (which is how I tested the patch, of course)
>
> Ah, right. IMO it'd be helpful to mention that echo example in your
> changes to psql-ref.sgml, either as part of your example inside the
> <programlisting>, or as a suggestion with the rest of the text.
>
> BTW, have you tested this on Windows? I don't have a Windows machine
> handy to fool with, but I do see what might be a mess/confusion on
> that platform. The MSDN docs claim[1] that putenv() is deprecated in
> favor of _putenv(). The code in pg_upgrade uses
> SetEnvironmentVariableA on WIN32, while win32env.c has a wrapper
> function pgwin32_putenv() around _putenv().

I also wondered this and also don't have a Windows build system.

The win32.h uses a macro to turn putenv() into pgwin32_putenv() , so
as long as that macro is in effect I think it should be doing the
right thing. In any event, there are several other places in the
existing code-base that call putenv() in a similar fashion to the new
code, so it if doesn't work I would expect things to already be
falling over.

>
> On Sat, Sep 24, 2011 at 5:18 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>> A description of the \setenv command should show up in the output of \?.
>
> Yeah, Andrew agreed upthread that help.c should be amended as well,
> which would fix \?.

Yes, sorry for the accidental nag. I didn't realize that help.c is
what implements \? until after I posted.

>
>> Should there be a regression test for this?  I'm not sure how it would
>> work, as I don't see a cross-platform way to see what the variable is
>> set to.
>
> Similar recent psql changes haven't had regression tests included, and
> I don't see much of a need here either.

I wasn't sure of the convention on that. I intentionally broke a few
\ commands (because that was easier than reading through all of
regress) and it did start failing, but that looks like that is because
those commands are used incidentally as part of other tests, rather
than having tests in their own right.

But in any case, considering that we are both wondering if it works on
Windows, I think that argues that an automatic regression test would
be very handy.

Cheers,

Jeff

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2011-09-26 21:16:01 Re: psql setenv command
Previous Message Andreas Karlsson 2011-09-26 21:03:07 EXECUTE tab completion