Re: proposal - assign result of query to psql variable

Hello

2012/9/19 Shigeru HANADA <shigeru.hanada@gmail.com>:
> On Fri, Aug 10, 2012 at 3:21 AM, Pavel Stehule <pavel.stehule@gmail.com>
> wrote:
>> there is new version of this patch
>>
>> * cleaned var list parser
>> * new regress tests
>> * support FETCH_COUNT > 0
>
> Here are my review comments.
>
> Submission
> ==========
> The patch is formatted in context diff style, and it could be applied
> cleanly against latest master.  This patch include document and tests,
> but IMO they need some enhancement.
>
> Usability
> =========
> This patch provides new psql command \gset which sends content of query
> buffer to server, and stores result of the query into psql variables.
> The name "\gset" is mixture of \g, which sends result to file or pipe,
> and \set, which sets variable to some value, so it would sound natural
> to psql users.
>
> Freature test
> =============
> Compile completed without warning.  Regression tests for \gset passed,
> but I have some comments on them.
>
> - Other regression tests have comment "-- ERROR" just after queries
> which should fail.  It would be nice to follow this manner.
> - Typo "to few" in expected file and source file.
> - How about adding testing "\gset" (no variable list) to "should fail"?
> - 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

> 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.

>
> 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

>
> Although I'll look the code more closely later, but anyway I marked the
> patch "Waiting on Author" for comments above.
>
> Regards,
> --
> Shigeru HANADA