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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers