proposal - assign result of query to psql variable

Pavel Stehule <pavel.stehule@gmail.com> writes:

\eset variable [, variable [..]] query -- it raise exception when
more than one row is returned or when no row is returned

Better would be a variant on \g, that is you type in the query and
then tell it where to put the result. We have learned the hard way
that putting SQL commands into the arguments of backslash commands
is a horrid idea. Maybe

select x,y,... from ...
\gset var1 var2 ...

regards, tom lane

2012/7/26 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

\eset variable [, variable [..]] query -- it raise exception when
more than one row is returned or when no row is returned

Better would be a variant on \g, that is you type in the query and
then tell it where to put the result. We have learned the hard way
that putting SQL commands into the arguments of backslash commands
is a horrid idea. Maybe

select x,y,... from ...
\gset var1 var2 ...

it could be

Pavel

On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:

Pavel Stehule <pavel.stehule@gmail.com> writes:

\eset variable [, variable [..]] query -- it raise exception when
more than one row is returned or when no row is returned

Better would be a variant on \g, that is you type in the query and
then tell it where to put the result. We have learned the hard way
that putting SQL commands into the arguments of backslash commands
is a horrid idea. Maybe

select x,y,... from ...
\gset var1 var2 ...

How about

\gset var1,,,var2,var3...

The above shows how one would skip assigning variables in the target
list, which one might want to do.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

2012/7/26 David Fetter <david@fetter.org>:

On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:

Pavel Stehule <pavel.stehule@gmail.com> writes:

\eset variable [, variable [..]] query -- it raise exception when
more than one row is returned or when no row is returned

Better would be a variant on \g, that is you type in the query and
then tell it where to put the result. We have learned the hard way
that putting SQL commands into the arguments of backslash commands
is a horrid idea. Maybe

select x,y,... from ...
\gset var1 var2 ...

How about

\gset var1,,,var2,var3...

I don't like this - you can use fake variable - and ignoring some
variable has no big effect on client

Pavel

On Thu, Jul 26, 2012 at 08:31:13AM +0200, Pavel Stehule wrote:

2012/7/26 David Fetter <david@fetter.org>:

On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:

Pavel Stehule <pavel.stehule@gmail.com> writes:

\eset variable [, variable [..]] query -- it raise exception when
more than one row is returned or when no row is returned

Better would be a variant on \g, that is you type in the query and
then tell it where to put the result. We have learned the hard way
that putting SQL commands into the arguments of backslash commands
is a horrid idea. Maybe

select x,y,... from ...
\gset var1 var2 ...

How about

\gset var1,,,var2,var3...

I don't like this - you can use fake variable - and ignoring some
variable has no big effect on client

Why assign to a variable you'll never use?

Cheers,
David.

P.S. The bike shed should be puce with blaze orange pin-striping.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

2012/7/26 David Fetter <david@fetter.org>:

On Thu, Jul 26, 2012 at 08:31:13AM +0200, Pavel Stehule wrote:

2012/7/26 David Fetter <david@fetter.org>:

On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:

Pavel Stehule <pavel.stehule@gmail.com> writes:

\eset variable [, variable [..]] query -- it raise exception when
more than one row is returned or when no row is returned

Better would be a variant on \g, that is you type in the query and
then tell it where to put the result. We have learned the hard way
that putting SQL commands into the arguments of backslash commands
is a horrid idea. Maybe

select x,y,... from ...
\gset var1 var2 ...

How about

\gset var1,,,var2,var3...

I don't like this - you can use fake variable - and ignoring some
variable has no big effect on client

Why assign to a variable you'll never use?

so why you get data from server, when you would not to use it ?

no offence, probably it is not hard to implement it - because we use
own parser, but I see this proposal little bit obscure

Tom - your proposal release of stored dataset just before next
statement, not like now on the end of statement?

Regards

Pavel

Pavel Stehule <pavel.stehule@gmail.com> writes:

2012/7/26 David Fetter <david@fetter.org>:

How about
\gset var1,,,var2,var3...

I don't like this - you can use fake variable - and ignoring some
variable has no big effect on client

Why assign to a variable you'll never use?

so why you get data from server, when you would not to use it ?

Yeah. I don't see why you'd be likely to write a select that computes
columns you don't actually want.

Tom - your proposal release of stored dataset just before next
statement, not like now on the end of statement?

Huh? I think you'd assign the values to the variables and then PQclear
the result right away.

regards, tom lane

Hello

2012/7/27 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

2012/7/26 David Fetter <david@fetter.org>:

How about
\gset var1,,,var2,var3...

I don't like this - you can use fake variable - and ignoring some
variable has no big effect on client

Why assign to a variable you'll never use?

so why you get data from server, when you would not to use it ?

Yeah. I don't see why you'd be likely to write a select that computes
columns you don't actually want.

Tom - your proposal release of stored dataset just before next
statement, not like now on the end of statement?

Huh? I think you'd assign the values to the variables and then PQclear
the result right away.

yes - I didn't understand \g mechanism well.

Here is patch - it is not nice at this moment and it is little bit
longer than I expected - but it works

It supports David's syntax

postgres=# select 'Hello', 'World' \gset a,b
postgres=# \echo :'a' :'b'
'Hello' 'World'
postgres=# select 'Hello', 'World';
?column? │ ?column?
──────────┼──────────
Hello │ World
(1 row)

postgres=# \gset a
to few target variables
postgres=# \gset a,
postgres=# \echo :'a'
'Hello'

Regards

Pavel

On Sat, Jul 28, 2012 at 06:11:21PM +0200, Pavel Stehule wrote:

Hello

2012/7/27 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

2012/7/26 David Fetter <david@fetter.org>:

How about
\gset var1,,,var2,var3...

I don't like this - you can use fake variable - and ignoring some
variable has no big effect on client

Why assign to a variable you'll never use?

so why you get data from server, when you would not to use it ?

Yeah. I don't see why you'd be likely to write a select that computes
columns you don't actually want.

Tom - your proposal release of stored dataset just before next
statement, not like now on the end of statement?

Huh? I think you'd assign the values to the variables and then PQclear
the result right away.

yes - I didn't understand \g mechanism well.

Here is patch - it is not nice at this moment and it is little bit
longer than I expected - but it works

It supports David's syntax

postgres=# select 'Hello', 'World' \gset a,b
postgres=# \echo :'a' :'b'
'Hello' 'World'
postgres=# select 'Hello', 'World';
?column? │ ?column?
──────────┼──────────
Hello │ World
(1 row)

postgres=# \gset a
to few target variables
postgres=# \gset a,
postgres=# \echo :'a'
'Hello'

Regards

Pavel

Teensy code cleanup (trailing space) and SGML docs added.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Hello

there is new version of this patch

* cleaned var list parser
* new regress tests
* support FETCH_COUNT > 0

Regards

Pavel Stehule

2012/8/1 David Fetter <david@fetter.org>:

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.

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

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.

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

Regards,
--
Shigeru HANADA

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

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

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

Hello

here is updated version of gset patch.

* merge Shigeru's doc patch
* rename psql regression test from "psql" to "psql_cmd"

Regards

Pavel Stehule

2012/9/27 Pavel Stehule <pavel.stehule@gmail.com>:

Hi Pavel,

On Sat, Oct 13, 2012 at 12:58 AM, Pavel Stehule
<pavel.stehule@gmail.com> wrote:

* merge Shigeru's doc patch
* rename psql regression test from "psql" to "psql_cmd"

Those changes seem good.

Besides, I found an error message which doesn't end with '¥n' in
common.c. In general, messages passed to psql_error end with '¥n',
unless additional information follows. Please see attached patch for
additional change.

After you determine whether it's ok or unnecessary, I'll mark this patch
as "Ready for committer".

Regards,
--
Shigeru HANADA

2012/10/13 Shigeru HANADA <shigeru.hanada@gmail.com>:

Hi Pavel,

On Sat, Oct 13, 2012 at 12:58 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

* merge Shigeru's doc patch
* rename psql regression test from "psql" to "psql_cmd"

Those changes seem good.

Besides, I found an error message which doesn't end with '¥n' in common.c.
In general, messages passed to psql_error end with '¥n', unless additional
information follows. Please see attached patch for additional change.

After you determine whether it's ok or unnecessary, I'll mark this patch as
"Ready for committer".

it is ok, thank you

Pavel

On Sat, October 13, 2012 19:26, Pavel Stehule wrote:

2012/10/13 Shigeru HANADA <shigeru.hanada@gmail.com>:

After you determine whether it's ok or unnecessary, I'll mark this patch as
"Ready for committer".

I found this behaviour which I think must count as a bug.
\gset doesn't allow more \\-separated lines behind it:

Only the last of these commands is problematic, and giving the syntax error

$ psql
psql (9.3devel-psql_var-20121012_2345-8b728e5c6e0ce6b6d6f54b92b390f14aa1aca6db)
Type "help" for help.

testdb=# select 1,2 \gset x,y
testdb=# \echo :x
1
testdb=# \echo :y
2
testdb=# \echo :x \\ \echo :y
1
2
testdb=# select 1,2 \gset x,y \\ \echo :x
\gset: syntax error
testdb=#

It'd be nice if it could be made to work

Thanks

Erik Rijkers

Hello

2012/10/14 Erik Rijkers <er@xs4all.nl>:

On Sat, October 13, 2012 19:26, Pavel Stehule wrote:

2012/10/13 Shigeru HANADA <shigeru.hanada@gmail.com>:

After you determine whether it's ok or unnecessary, I'll mark this patch as
"Ready for committer".

I found this behaviour which I think must count as a bug.
\gset doesn't allow more \\-separated lines behind it:

Only the last of these commands is problematic, and giving the syntax error

$ psql
psql (9.3devel-psql_var-20121012_2345-8b728e5c6e0ce6b6d6f54b92b390f14aa1aca6db)
Type "help" for help.

testdb=# select 1,2 \gset x,y
testdb=# \echo :x
1
testdb=# \echo :y
2
testdb=# \echo :x \\ \echo :y
1
2
testdb=# select 1,2 \gset x,y \\ \echo :x
\gset: syntax error
testdb=#

It'd be nice if it could be made to work

done

Regards

Pavel