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.
>> 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.
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?
--
Shigeru HANADA
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 3693a5a..c4ac674 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1483,8 +1483,8 @@ testdb=>
way. Use <command>\i</command> for that.) This means that
if the query ends with (or contains) a semicolon, it is
immediately executed. Otherwise it will merely wait in the
- query buffer; type semicolon, <literal>\g</> or <literal>\gset</literal> to send it, or
- <literal>\r</> to cancel.
+ query buffer; type semicolon, <literal>\g</> or
+ <literal>\gset</literal> to send it, or <literal>\r</> to cancel.
</para>
<para>
@@ -1621,9 +1621,19 @@ Tue Oct 26 21:40:57 CEST 1999
<listitem>
<para>
- Sends the current query input buffer to the server and stores
- the query's target list a corresponding list of psql
- variables.
+ Sends the current query input buffer to the server and stores the
+ query's output into corresponding <replaceable
+ class="parameter">variable</replaceable>. The preceding query must
+ return only one row, and the number of variables must be same as the
+ number of elements in <command>SELECT</command> list. If you don't
+ need any of items in <command>SELECT</command> list, you can omit
+ corresponding <replaceable class="parameter">variable</replaceable>.
+ Example:
+<programlisting>
+foo=> SELECT 'hello', 'wonderful', 'world!' \gset var1,,var3
+foo=> \echo :var1 :var3
+hello world!
+</programlisting>
</para>
</listitem>
</varlistentry>
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers