Re: proposal - assign result of query to psql variable - Mailing list pgsql-hackers

Hello

2012/9/21 Shigeru HANADA <shigeru.hanada@gmail.com>:
> Hi Pavel,
>
> (2012/09/21 2:01), Pavel Stehule wrote:
>>> - Is it intentional that \gset can set special variables such as
>>> AUTOCOMMIT and HOST?  I don't see any downside for this behavior,
>>> because \set also can do that, but it is not documented nor tested at all.
>>>
>>
>> I use a same "SetVariable" function, so a behave should be same
>
> It seems reasonable.
>
>>> Document
>>> ========
>>> - Adding some description of \gset command, especially about limitation
>>> of variable list, seems necessary.
>>> - In addition to the meta-command section, "Advanced features" section
>>> mentions how to set psql's variables, so we would need some mention
>>> there too.
>>> - The term "target list" might not be familiar to users, since it
>>> appears in only sections mentioning PG internal relatively.  I think
>>> that the feature described in the section "Retrieving Query Results" in
>>> ECPG document is similar to this feature.
>>> http://www.postgresql.org/docs/devel/static/ecpg-variables.html
>>
>> I invite any proposals about enhancing documentation. Personally I am
>> a PostgreSQL developer, so I don't known any different term other than
>> "target list" - but any user friendly description is welcome.
>
> How about to say "stores the query's result output into variable"?
> Please see attached file for my proposal.  I also mentioned about 1-row
> limit and omit of variable.

should be

>
>>> Coding
>>> ======
>>> The code follows our coding conventions.  Here are comments for coding.
>>>
>>> - Some typo found in comments, please see attached patch.
>>> - There is a code path which doesn't print error message even if libpq
>>> reported error (PGRES_BAD_RESPONSE, PGRES_NONFATAL_ERROR,
>>> PGRES_FATAL_ERROR) in StoreQueryResult.  Is this intentional?  FYI, ecpg
>>> prints "bad response" message for those errors.
>>
>> yes - it is question. I use same pattern like PrintQueryResult, but
>> "bad response" message should be used.
>>
>> I am sending updated patch
>
> It seems ok.
>
> BTW, as far as I see, no psql backslash command including \setenv (it
> was added in 9.2) has regression test in core (I mean src/test/regress).
>  Is there any convention about this issue?  If psql backslash commands
> (or any psql feature else) don't need regression test, we can remove
> psql.(sql|out).
> # Of course we need to test new feature by hand.

It is question for Tom or David - only server side functionalities has
regress tests. But result of some backslash command is verified in
other regress tests. I would to see some regression tests for this
functionality.

>
> Anyway, IMO the name psql impresses larger area than the patch
> implements.  How about to rename psql to psql_cmd or backslash_cmd than
> psql as regression test name?
>

I have no idea - psql_cmd is good name

Regards

Pavel

> --
> Shigeru HANADA