Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-03 Thread Pavel Stehule
2013/2/2 Tom Lane t...@sss.pgh.pa.us:
 Shigeru Hanada shigeru.han...@gmail.com writes:
 On Sat, Feb 2, 2013 at 7:30 PM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 possible variants

 a) don't store NULL  values - and remove existing variable when NULL
 be assigned - it is probably best, but should be confusing for users
 b) implement flag IS NULL - for variables
 c) use nullPrint
 d) use empty String

 +1 for a).  If users want to determine whether the result was NULL, or
 want to use substitute string for NULL result, they can use coalesce
 in SELECT clause.  Anyway the feature should be documented clearly.

 Yeah, I was considering that one too.  Let's do it that way.

 regards, tom lane

possible simple enhancing of this behave (for 9.4).

now missing variables is replaced by variable's name. We can implement
some pset option - some like define what do with missing variable

\pset missing_variable (use_name | use_null | error )

when this option will be active, then missing variable will be
replaced by NULL. With this feature sequences of SQL statements joined
by some variables can work.

SELECT NULL as myvar \gset
\pset missing_variable use_null
SELECT :'myvar' IS NULL;

ideas, opinions ?

Regards

Pavel


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-03 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 now missing variables is replaced by variable's name. We can implement
 some pset option - some like define what do with missing variable

 \pset missing_variable (use_name | use_null | error )

No, it isn't replaced by variable's name.  What actually happens is we
don't attempt a replacement unless the string after the colon matches an
existing variable.  Tampering with that seems dangerous and foolish.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-03 Thread Pavel Stehule
2013/2/3 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 now missing variables is replaced by variable's name. We can implement
 some pset option - some like define what do with missing variable

 \pset missing_variable (use_name | use_null | error )

 No, it isn't replaced by variable's name.  What actually happens is we
 don't attempt a replacement unless the string after the colon matches an
 existing variable.  Tampering with that seems dangerous and foolish.

some other ideas?

do you think so full NULL support has sense?

Regards

Pavel


 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-02 Thread Pavel Stehule
Hello

2013/2/1 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 here is patch related to your proposal

 I looked at this a bit.  It's getting there, though I still don't trust
 the places where you've chosen to clear the prefix setting.  (Looking at
 it, I'm now not sure that I trust the implementation of \g either.)

 However, what I wanted to ask about was this:

 + if (PQgetisnull(result, 0, i))
 + value = pset.popt.nullPrint ? 
 pset.popt.nullPrint : ;
 + else
 + value = PQgetvalue(result, 0, i);

 What's the argument for using nullPrint here?  ISTM that's effectively a
 form of escaping, and I'd not expect that to get applied to values going
 into variables, any more than any other formatting we do when printing
 results.

 Admittedly, if we just take the PQgetvalue result blindly, there'll
 be no way to tell the difference between empty-string and NULL results.
 But I'm not convinced that this approach is better.  It would certainly
 need more than no documentation.


I have not strong opinion about storing NULL value - and nullPrint is
a best from simple variants -

possible variants

a) don't store NULL  values - and remove existing variable when NULL
be assigned - it is probably best, but should be confusing for users
b) implement flag IS NULL - for variables
c) use nullPrint
d) use empty String

@d is subset of @c, and I think so it put some better possibilities
with only two lines more

@a is probably best - but significant change - not hard to implement it

SELECT NULL AS x \g pref_
SELECT :'pref_' IS NULL;

is can be nice

but it should be premature optimization too - nullPrint is enough for
typical use cases

Regards

Pavel

Regards

Pavel




 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-02 Thread Shigeru Hanada
On Sat, Feb 2, 2013 at 7:30 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 possible variants

 a) don't store NULL  values - and remove existing variable when NULL
 be assigned - it is probably best, but should be confusing for users
 b) implement flag IS NULL - for variables
 c) use nullPrint
 d) use empty String

+1 for a).  If users want to determine whether the result was NULL, or
want to use substitute string for NULL result, they can use coalesce
in SELECT clause.  Anyway the feature should be documented clearly.

-- 
Shigeru HANADA


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-02 Thread Pavel Stehule
2013/2/2 Shigeru Hanada shigeru.han...@gmail.com:
 On Sat, Feb 2, 2013 at 7:30 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 possible variants

 a) don't store NULL  values - and remove existing variable when NULL
 be assigned - it is probably best, but should be confusing for users
 b) implement flag IS NULL - for variables
 c) use nullPrint
 d) use empty String

 +1 for a).  If users want to determine whether the result was NULL, or
 want to use substitute string for NULL result, they can use coalesce
 in SELECT clause.  Anyway the feature should be documented clearly.


yes, this has one other advantage - it doesn't block possible
enhancing variables about NULL support in future. And other - it
doesn't depends on psql settings

Regards

Pavel

 --
 Shigeru HANADA


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-02 Thread Tom Lane
Shigeru Hanada shigeru.han...@gmail.com writes:
 On Sat, Feb 2, 2013 at 7:30 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 possible variants
 
 a) don't store NULL  values - and remove existing variable when NULL
 be assigned - it is probably best, but should be confusing for users
 b) implement flag IS NULL - for variables
 c) use nullPrint
 d) use empty String

 +1 for a).  If users want to determine whether the result was NULL, or
 want to use substitute string for NULL result, they can use coalesce
 in SELECT clause.  Anyway the feature should be documented clearly.

Yeah, I was considering that one too.  Let's do it that way.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-02 Thread Pavel Stehule
Hello

2013/2/2 Tom Lane t...@sss.pgh.pa.us:
 Shigeru Hanada shigeru.han...@gmail.com writes:
 On Sat, Feb 2, 2013 at 7:30 PM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 possible variants

 a) don't store NULL  values - and remove existing variable when NULL
 be assigned - it is probably best, but should be confusing for users
 b) implement flag IS NULL - for variables
 c) use nullPrint
 d) use empty String

 +1 for a).  If users want to determine whether the result was NULL, or
 want to use substitute string for NULL result, they can use coalesce
 in SELECT clause.  Anyway the feature should be documented clearly.

 Yeah, I was considering that one too.  Let's do it that way.

updated version

Regards

Pavel


 regards, tom lane


gset_20130202.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-02 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 2013/2/2 Tom Lane t...@sss.pgh.pa.us:
 Shigeru Hanada shigeru.han...@gmail.com writes:
 +1 for a).  If users want to determine whether the result was NULL, or
 want to use substitute string for NULL result, they can use coalesce
 in SELECT clause.  Anyway the feature should be documented clearly.

 Yeah, I was considering that one too.  Let's do it that way.

 updated version

Applied with corrections.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-02-01 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 here is patch related to your proposal

I looked at this a bit.  It's getting there, though I still don't trust
the places where you've chosen to clear the prefix setting.  (Looking at
it, I'm now not sure that I trust the implementation of \g either.)

However, what I wanted to ask about was this:

 + if (PQgetisnull(result, 0, i))
 + value = pset.popt.nullPrint ? 
 pset.popt.nullPrint : ;
 + else
 + value = PQgetvalue(result, 0, i);

What's the argument for using nullPrint here?  ISTM that's effectively a
form of escaping, and I'd not expect that to get applied to values going
into variables, any more than any other formatting we do when printing
results.

Admittedly, if we just take the PQgetvalue result blindly, there'll
be no way to tell the difference between empty-string and NULL results.
But I'm not convinced that this approach is better.  It would certainly
need more than no documentation.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-01-31 Thread Pavel Stehule
Hello

can you look, please, on updated version - it respects Tom's proposal
and it is significantly reduced?

Thank you

Pavel Stehule

2013/1/28 Pavel Stehule pavel.steh...@gmail.com:
 Hello

 2013/1/26 Tom Lane t...@sss.pgh.pa.us:
 Andrew Dunstan and...@dunslane.net writes:
 +1. This looks quite nifty. Maybe useful too to have a default prefix
 via some setting.

 Meh.  I would expect that \gset :foo would work to specify a computed
 prefix if you wanted it --- isn't that sufficient indirection?  I'm not
 thrilled with further expanding the set of magic variables in psql.


 here is patch related to your proposal

 Regards

 Pavel

 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-01-28 Thread Pavel Stehule
Hello

2013/1/26 Tom Lane t...@sss.pgh.pa.us:
 Andrew Dunstan and...@dunslane.net writes:
 +1. This looks quite nifty. Maybe useful too to have a default prefix
 via some setting.

 Meh.  I would expect that \gset :foo would work to specify a computed
 prefix if you wanted it --- isn't that sufficient indirection?  I'm not
 thrilled with further expanding the set of magic variables in psql.


here is patch related to your proposal

Regards

Pavel

 regards, tom lane


gset_20130128.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-01-26 Thread Tom Lane
I wrote:
 Pavel Stehule pavel.steh...@gmail.com writes:
 [ gset_13.diff ]

 One more gripe is that the parsing logic for \gset is pretty
 unintelligible.

After further thought, it seems to me that the problem here is an
overcomplicated definition of the \gset command; it could be made
both more usable and simpler to implement, if we looked at it
differently.

First off, why is there a provision to omit variable names for some
columns, ie why bother with saying that you can write \gset x,,y to
store only the first and third columns?  If you didn't want the second
value, why didn't you leave it out of the SELECT to start with?
It seems like the only possible reason for that is if you were lazy
and typed SELECT * instead of listing the columns ... but then you
still need to list the columns in \gset, and it's pretty error-prone
to make sure that the \gset variable list lines up with what * will
emit.

In fact, it's pretty error-prone to have to make the \gset variable list
line up with the SELECT columns in any case.  So here's my proposal:
let's forget the variable list entirely, and use the column names
returned by the server as the variable names to assign to.  So instead
of
select 1, 2 \gset x,y
you would write
select 1 as x, 2 as y \gset
or just
select 1 x, 2 y \gset
which is exactly as much typing as the existing definition, but much
harder to screw up by misaligning the SELECT's values with the target
names.  It also makes it really trivial to do the SELECT * case ---
you just do it, and ignore any variables for columns you don't care
about.

A probably-useful extension to this basic concept is to allow \gset
to specify an optional prefix, that is
select 1 as x, 2 as y \gset p_
would set p_x and p_y.  This would make it easier to manage results from
multiple \gset operations, and to be sure that you didn't accidentally
overwrite some built-in variable.

So this seems to me to be easier and less error-prone to use than the
existing definition.  It would also take a lot less code to implement,
since the parsing logic for \gset would reduce to a couple of lines,
and you'd not need the variable-name-list data structure at all.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-01-26 Thread Andrew Dunstan


On 01/26/2013 11:42 AM, Tom Lane wrote:


A probably-useful extension to this basic concept is to allow \gset
to specify an optional prefix, that is
select 1 as x, 2 as y \gset p_
would set p_x and p_y.  This would make it easier to manage results from
multiple \gset operations, and to be sure that you didn't accidentally
overwrite some built-in variable.


+1. This looks quite nifty. Maybe useful too to have a default prefix 
via some setting.



cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-01-26 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 +1. This looks quite nifty. Maybe useful too to have a default prefix 
 via some setting.

Meh.  I would expect that \gset :foo would work to specify a computed
prefix if you wanted it --- isn't that sufficient indirection?  I'm not
thrilled with further expanding the set of magic variables in psql.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-01-26 Thread Pavel Stehule
Hello

2013/1/26 Tom Lane t...@sss.pgh.pa.us:
 I wrote:
 Pavel Stehule pavel.steh...@gmail.com writes:
 [ gset_13.diff ]

 One more gripe is that the parsing logic for \gset is pretty
 unintelligible.

 After further thought, it seems to me that the problem here is an
 overcomplicated definition of the \gset command; it could be made
 both more usable and simpler to implement, if we looked at it
 differently.

 First off, why is there a provision to omit variable names for some
 columns, ie why bother with saying that you can write \gset x,,y to
 store only the first and third columns?  If you didn't want the second
 value, why didn't you leave it out of the SELECT to start with?
 It seems like the only possible reason for that is if you were lazy
 and typed SELECT * instead of listing the columns ... but then you
 still need to list the columns in \gset, and it's pretty error-prone
 to make sure that the \gset variable list lines up with what * will
 emit.

possibility to skip some variables is David Fetter's idea. I see only
one possible use case - it enable use some query from history without
necessity to modify query or creating some auxiliary variables.
Personally, I can live without this feature, but it question for David
mainly.


 In fact, it's pretty error-prone to have to make the \gset variable list
 line up with the SELECT columns in any case.  So here's my proposal:
 let's forget the variable list entirely, and use the column names
 returned by the server as the variable names to assign to.  So instead
 of
 select 1, 2 \gset x,y
 you would write
 select 1 as x, 2 as y \gset
 or just
 select 1 x, 2 y \gset
 which is exactly as much typing as the existing definition, but much
 harder to screw up by misaligning the SELECT's values with the target
 names.  It also makes it really trivial to do the SELECT * case ---
 you just do it, and ignore any variables for columns you don't care
 about.

hard to say

your proposal has advantages - and implementation is simple, but it is
looking little bit strange - but like other psql features.

I have no strong opinion - I prefer original design, as more explicit
with clean separation line between query and console part, but I am
able to see advantages of your proposal - so depends what will speak
others. I have no problem with your design, although I am thinking so
original design is little bit more safer (but not with significant
difference).


 A probably-useful extension to this basic concept is to allow \gset
 to specify an optional prefix, that is
 select 1 as x, 2 as y \gset p_
 would set p_x and p_y.  This would make it easier to manage results from
 multiple \gset operations, and to be sure that you didn't accidentally
 overwrite some built-in variable.

I understand to motivation - but I am not enthused. Now - a work with
variables is strange - and with it will be more stranger.


 So this seems to me to be easier and less error-prone to use than the
 existing definition.  It would also take a lot less code to implement,
 since the parsing logic for \gset would reduce to a couple of lines,
 and you'd not need the variable-name-list data structure at all.

I will waiting for others - I can live with this proposal.

Regards

Pavel


 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2013-01-25 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 [ gset_13.diff ]

I looked at this a bit.  I think you need to reconsider when and how the
\gset state gets cleaned up.  Doing it inside StoreQueryResult is not
very good because that only gets reached upon successful execution.
Consider for example

select 1/0 \gset x

You'll get an ERROR from this, and a reasonable user would suppose that
that was that and the \gset is no longer in effect.  But guess what,
it's still lurking under the surface, waiting to capture his next command.

This is also causing you unnecessary complication in
ExecQueryUsingCursor, which has to work around the fact that
StoreQueryResult destroys state.

I think it would be better to remove that responsibility from
StoreQueryResult and instead put the gset-list cleanup somewhere at the
end of query processing.  Didn't really look into where would be the
best place, but it should be someplace that control passes through no
matter what came back from the server.

BTW, is StoreQueryResult in itself (that is, the switch on
PQresultStatus) actually doing anything useful?  It appears to me that
the error cases are handled elsewhere, such that control never gets to
it unless the PQresultStatus is an expected value.  If that were not the
case, printouts as laconic as bad response\n would certainly not be
acceptable --- people would want to see the underlying error message.

Also, I'm not sure I like the PSQL_CMD_NOSEND business, ie, trashing
the query buffer if anything can be found wrong with the \gset itself.
If I've done

big long multiline query here
\gset x y

I'd expect that the error only discards the \gset and not the query.
An error in some other sort of backslash command in that situation
wouldn't discard the query buffer.  For instance try this:

regression=# select 3+2
regression-# \goofus
Invalid command \goofus. Try \? for help.
regression-# ;
 ?column? 
--
5
(1 row)

regression=# 

So AFAICS, PSQL_CMD_NOSEND just represents extra code that's making
things worse not better.

One more gripe is that the parsing logic for \gset is pretty
unintelligible.  You've got a state variable with only two values,
whose names seem to boil down to whether a comma is expected or not.
And then you've got a separate process_comma flag, which means
... well, I'm not sure, but possibly the very same thing.  For sure it's
not clear whether all four possible combinations of those two variables
are valid and what they'd mean.  This could use another round of
thinking and rewriting.  Or at least better comments.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-12-19 Thread Pavel Stehule
2012/12/19 Shigeru Hanada shigeru.han...@gmail.com:
 On Tue, Dec 18, 2012 at 2:52 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 Attached updated patch

 Seems fine.  I'll mark this as ready for committer.

 Nice work!

thank you very much

Regards

Pavel


 --
 Shigeru HANADA


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-12-18 Thread Shigeru Hanada
On Tue, Dec 18, 2012 at 2:52 AM, Pavel Stehule pavel.steh...@gmail.com wrote:
 Attached updated patch

Seems fine.  I'll mark this as ready for committer.

Nice work!

-- 
Shigeru HANADA


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-12-17 Thread Shigeru Hanada
On Sun, Oct 28, 2012 at 7:16 AM, Pavel Stehule pavel.steh...@gmail.com wrote:
 Hello

 here is updated patch

 main change - it doesn't touch psql lexer - like Tom proposed
 other points respect Phil's notices

I reviewed v12 patch.  It provides gset command which works
consistently with other psql commands, such as \g and \set, and
implementation seems reasonable, and follows other reviewer's comments
properly.  I think we can mark it as ready for committer, once you
have fixed some minor issues below.

* Skipping leading blank in inner while loop of command.c seems
unnecessary, because (IIUC) psql's scanner skips blanks.  Is there any
case that scanner returns token with leading/trailing blank?
* ISTM that VARLIST_INITIAL can be removed.  AFAIS it's same state as
VARLIST_EXPECTED_COMMA_OR_IDENT.
* I found some cosmetic flaw and typo.  Please see attached patch for details.
* How about pulling up codes for PGRES_TUPLES_OK case in
StoreQueryResult to new static function, say StoreQueryTuple?  It
would make StoreQueryResult more similar to PrintQueryResult's style,
and IMO it makes the code more readable.

Regards,
-- 
Shigeru HANADA


gset_fix.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-12-17 Thread Pavel Stehule
2012/12/17 Shigeru Hanada shigeru.han...@gmail.com:
 On Sun, Oct 28, 2012 at 7:16 AM, Pavel Stehule pavel.steh...@gmail.com 
 wrote:
 Hello

 here is updated patch

 main change - it doesn't touch psql lexer - like Tom proposed
 other points respect Phil's notices

 I reviewed v12 patch.  It provides gset command which works
 consistently with other psql commands, such as \g and \set, and
 implementation seems reasonable, and follows other reviewer's comments
 properly.  I think we can mark it as ready for committer, once you
 have fixed some minor issues below.

 * Skipping leading blank in inner while loop of command.c seems
 unnecessary, because (IIUC) psql's scanner skips blanks.  Is there any
 case that scanner returns token with leading/trailing blank?

removed

 * ISTM that VARLIST_INITIAL can be removed.  AFAIS it's same state as
 VARLIST_EXPECTED_COMMA_OR_IDENT.

removed

 * I found some cosmetic flaw and typo.  Please see attached patch for details.

it is ok, merged

 * How about pulling up codes for PGRES_TUPLES_OK case in
 StoreQueryResult to new static function, say StoreQueryTuple?  It
 would make StoreQueryResult more similar to PrintQueryResult's style,
 and IMO it makes the code more readable.

good idea
done

Attached updated patch

Regards

Pavel


 Regards,
 --
 Shigeru HANADA


gset_13.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-28 Thread Pavel Stehule
Hello

here is updated patch

main change - it doesn't touch psql lexer - like Tom proposed
other points respect Phil's notices




 My first review...

 Patch applied cleanly to master and make check was fine.

 I tested it out and it operates as advertised. There were a couple
 things that stood out to me though.

 1) NULL values are not displayed properly after \pset null is run.

 postgres=# \pset null '(null)'
 Null display is (null).
 postgres=# select NULL \gset var1
 postgres=# \echo :var1

 postgres=# select NULL;
  ?column?
 --
  (null)
 (1 row)

 I know this doesn't come back from the server like this, but you
 should be able to pull this from the options and display
 appropriately. Not sure if it should be in variable display code, or
 when you store it into the variable.

fixed and add to regress test


 2) The error messages seemed kind of terse. Other error messages are
 capitalized and have punctuation.

After some thinking I didn't change it - it is consistent with other
messages in psql - short messages that are not complete sentence are
no capitalized and have not punctuation like other short messages in
psql


 3) We don't find out about incorrect number of columns until after
 query returns. I know this is hard/impossible to fix, but it might be
 nice to print out the result normally if you can't store it in the
 variables.

I didn't change it because a) I don't think so change behave after
error is good idea, b) \gset doesn't remove SQL from query buffer, so
you can repeat it

postgres= select 10,20,30
postgres- \gset a,b
too few target variables
postgres= \g
 ?column? │ ?column? │ ?column?
──┼──┼──
   10 │   20 │   30
(1 row)

postgres= \gset a,b,c



 3b) You throw an error on too many variables, but still store the data
 since you have fewer columns than variables. This makes sense, but you
 don't inform the user at all.

it is commented in doc

+ para
+  When this command fails, then related replaceable
+  class=parametervariables/replaceable has undefined content.
+ /para


 On to the code:

 1) Introduction of random newlines:

 *** cleanup:
 *** 1254,1259 
 --- 1383,1389 
 PQclear(results);
 }

 +
 if (pset.timing)
 {
 INSTR_TIME_SET_CURRENT(after);

 I saw that in a couple places, but that was the most obvious.


I hope so I moved to /dev/null all

 2) TargetList - Why not use the built in linked list operations rather
 than creating your own? Are they not accessible to client binaries
 like this?

There was not support for lists on client part to today. So I had to
created own simple implementation.


 Overall I think this is a useful feature and I think you integrated it
 well within the existing infrastructure, ie combining concepts of \set
 and \g.

Thank you very much again

Regards

Pavel


gset_12.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-25 Thread Alvaro Herrera
I gave this a look.  I think it needs to be revised by somebody with a
better understanding of scanner (flex) than me, but I didn't like the
changes in psqlscan.l at all; the new xvl pattern is too unlike the
rest of the surrounding patterns, and furthermore it has been placed
within the block that says it mirrors the backend scanner, when it
obviously has no equivalent there.

I assume there's a better way to do this.  Hints would be appreciated.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-25 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes:
 I gave this a look.  I think it needs to be revised by somebody with a
 better understanding of scanner (flex) than me, but I didn't like the
 changes in psqlscan.l at all; the new xvl pattern is too unlike the
 rest of the surrounding patterns, and furthermore it has been placed
 within the block that says it mirrors the backend scanner, when it
 obviously has no equivalent there.

 I assume there's a better way to do this.  Hints would be appreciated.

Personally I saw no reason for this patch to touch psqlscan.l in the
first place.  Commands such as \set just scan variable names with
psql_scan_slash_option(OT_NORMAL); why shouldn't this act the same?

Moreover, the proposed lexer rules are flat out *wrong*, in that they
insist on the target variable names being {identifier}s, a restriction
not imposed by \set.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-25 Thread Alvaro Herrera
Tom Lane escribió:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
  I gave this a look.  I think it needs to be revised by somebody with a
  better understanding of scanner (flex) than me, but I didn't like the
  changes in psqlscan.l at all; the new xvl pattern is too unlike the
  rest of the surrounding patterns, and furthermore it has been placed
  within the block that says it mirrors the backend scanner, when it
  obviously has no equivalent there.
 
  I assume there's a better way to do this.  Hints would be appreciated.
 
 Personally I saw no reason for this patch to touch psqlscan.l in the
 first place.  Commands such as \set just scan variable names with
 psql_scan_slash_option(OT_NORMAL); why shouldn't this act the same?
 
 Moreover, the proposed lexer rules are flat out *wrong*, in that they
 insist on the target variable names being {identifier}s, a restriction
 not imposed by \set.

Great, thanks for the feedback.  Marking as returned in CF.  I hope to
see a new version after pgconf.eu.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-25 Thread Pavel Stehule
2012/10/25 Tom Lane t...@sss.pgh.pa.us:
 Alvaro Herrera alvhe...@2ndquadrant.com writes:
 I gave this a look.  I think it needs to be revised by somebody with a
 better understanding of scanner (flex) than me, but I didn't like the
 changes in psqlscan.l at all; the new xvl pattern is too unlike the
 rest of the surrounding patterns, and furthermore it has been placed
 within the block that says it mirrors the backend scanner, when it
 obviously has no equivalent there.

 I assume there's a better way to do this.  Hints would be appreciated.

 Personally I saw no reason for this patch to touch psqlscan.l in the
 first place.  Commands such as \set just scan variable names with
 psql_scan_slash_option(OT_NORMAL); why shouldn't this act the same?


it cannot be same, because current scan doesn't know comma as
separator. So if you don't like changes in scanner, than we can't to
use var1, var2, syntax and we can't to use leaky list syntax ,x,

 Moreover, the proposed lexer rules are flat out *wrong*, in that they
 insist on the target variable names being {identifier}s, a restriction
 not imposed by \set.


do you like to support referenced varnames??

postgres=# \varname xxx
Invalid command \varname. Try \? for help.
postgres=# \set varname xxx
postgres=# \set :varname Hello
postgres=# \set
varname = 'xxx'
xxx = 'Hello'

yes, \set support it, but this can be source of strange behave for
some people, because people use :varname like $varname in classic
scripting languages, and it is significantly different - so I didn't
support it as little bit dangerous feature. It is easy support it,
although I am thinking, so it is not good idea, because behave is
really different than users expect and I don't know any use case for
this indirect referencing. But I would to talk about it, and I invite
opinion of others.

Regards

Pavel


 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-25 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 2012/10/25 Tom Lane t...@sss.pgh.pa.us:
 Personally I saw no reason for this patch to touch psqlscan.l in the
 first place.  Commands such as \set just scan variable names with
 psql_scan_slash_option(OT_NORMAL); why shouldn't this act the same?

 it cannot be same, because current scan doesn't know comma as
 separator. So if you don't like changes in scanner, than we can't to
 use var1, var2, syntax and we can't to use leaky list syntax ,x,

Uh, no, that doesn't follow.  It wouldn't be any more code to have
command.c process the commas (or even more likely, just save the \gset
argument(s) as a string, and split on commas after we've done the
command).  Even if we wanted to do that in psqlscan.l, this was a pretty
bad/ugly implementation of it.

 Moreover, the proposed lexer rules are flat out *wrong*, in that they
 insist on the target variable names being {identifier}s, a restriction
 not imposed by \set.

 yes, \set support it, but this can be source of strange behave for
 some people, because people use :varname like $varname in classic
 scripting languages, and it is significantly different - so I didn't
 support it as little bit dangerous feature.

[ shrug... ]  If you want to argue for imposing a restriction on
psql variable names across-the-board, we could have that discussion;
but personally I've not seen even one user complaint that could be
traced to \set's laxity in the matter, so I don't see a need for
a restriction.  In any case, having \gset enforce a restriction
that \set doesn't is useless and inconsistent.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-25 Thread Pavel Stehule
2012/10/25 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 2012/10/25 Tom Lane t...@sss.pgh.pa.us:
 Personally I saw no reason for this patch to touch psqlscan.l in the
 first place.  Commands such as \set just scan variable names with
 psql_scan_slash_option(OT_NORMAL); why shouldn't this act the same?

 it cannot be same, because current scan doesn't know comma as
 separator. So if you don't like changes in scanner, than we can't to
 use var1, var2, syntax and we can't to use leaky list syntax ,x,

 Uh, no, that doesn't follow.  It wouldn't be any more code to have
 command.c process the commas (or even more likely, just save the \gset
 argument(s) as a string, and split on commas after we've done the
 command).  Even if we wanted to do that in psqlscan.l, this was a pretty
 bad/ugly implementation of it.

I don't understand, why we have to move lexer work from scanner to
command processing?

then I afraid of another issue - when we do late separation in command

somebody can do

\set targetvars a,b,c

select 
\gset x1,x2,:targetvars,x3

We would to do this? Then we moving to TeX liked languages. I am asking.


 Moreover, the proposed lexer rules are flat out *wrong*, in that they
 insist on the target variable names being {identifier}s, a restriction
 not imposed by \set.

 yes, \set support it, but this can be source of strange behave for
 some people, because people use :varname like $varname in classic
 scripting languages, and it is significantly different - so I didn't
 support it as little bit dangerous feature.

 [ shrug... ]  If you want to argue for imposing a restriction on
 psql variable names across-the-board, we could have that discussion;
 but personally I've not seen even one user complaint that could be
 traced to \set's laxity in the matter, so I don't see a need for
 a restriction.  In any case, having \gset enforce a restriction
 that \set doesn't is useless and inconsistent.

ok, it has a sense


 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-24 Thread Phil Sorber
On Tue, Oct 16, 2012 at 12:24 PM, Pavel Stehule pavel.steh...@gmail.com wrote:
 2012/10/16 Shigeru HANADA shigeru.han...@gmail.com:
 Hi Pavel,

 On Tue, Oct 16, 2012 at 6:59 AM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
 here is updated patch, I moved lot of code from lexer to command.com,
 and now more \gset doesn't disable other backslash commands on same
 line.

 * lexer changes
 IIUC, new function psql_scan_varlist_varname scans input and returns a
 variable name or a comma at each call, and command.c handles the error
 such as invalid # of variables.  This new design seems better than old one.

 However, IMHO the name psql_scan_varlist_varname sounds redundant and
 unintuitive.  I'd prefer psql_scan_slash_varlist, because it indicates
 that that function is expected to be used for arguments of backslash
 commands, like psql_scan_slash_command and psql_scan_slash_option.
 Thoughts?

 * multiple meta command
 Now both of the command sequences

   $ SELECT 1, 2 \gset var1, var2 \g foo.txt
   $ SELECT 1, 2 \g foo.txt \gset var1, var2

 set var1 and v2 to 1 and 2 respectively, and also write the result
 into foo.txt.  This would be what users expected.

 * Duplication of variables
 I found an issue we have not discussed.  Currently \gset accepts same
 variable names in the list, and stores last SELECT item in duplicated
 variables.  For instance,

   $ SELECT 1, 2 \gset var, var

 stores 2 into var.  I think this behavior is acceptable, but it might
 be worth mentioning in document.

 * extra fixes
 I fixed some minor issues below.  Please see attached v10 patch for details.

   * remove unused macro OT_VARLIST
   * remove unnecessary #include directive for common.h
   * fill comment within 80 columns
   * indent short variable name with tab
   * add regression test case for combination of \g and \gset

 * bug on FETCH_COUNT = 1
 When FETCH_COUNT is set to 1, and the number of rows returned is 1 too,
 \gset shows extra (1 row).  This would be a bug in
 ExecQueryUsingCursor.  Please see the last test case in regression test
 psql_cmd.

 I fixed this bug

 Regards

 Pavel


 I'll mark this patch as waiting author.

 Regards,
 --
 Shigeru HANADA


 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers


My first review...

Patch applied cleanly to master and make check was fine.

I tested it out and it operates as advertised. There were a couple
things that stood out to me though.

1) NULL values are not displayed properly after \pset null is run.

postgres=# \pset null '(null)'
Null display is (null).
postgres=# select NULL \gset var1
postgres=# \echo :var1

postgres=# select NULL;
 ?column?
--
 (null)
(1 row)

I know this doesn't come back from the server like this, but you
should be able to pull this from the options and display
appropriately. Not sure if it should be in variable display code, or
when you store it into the variable.

2) The error messages seemed kind of terse. Other error messages are
capitalized and have punctuation.

3) We don't find out about incorrect number of columns until after
query returns. I know this is hard/impossible to fix, but it might be
nice to print out the result normally if you can't store it in the
variables.

3b) You throw an error on too many variables, but still store the data
since you have fewer columns than variables. This makes sense, but you
don't inform the user at all.

On to the code:

1) Introduction of random newlines:

*** cleanup:
*** 1254,1259 
--- 1383,1389 
PQclear(results);
}

+
if (pset.timing)
{
INSTR_TIME_SET_CURRENT(after);

I saw that in a couple places, but that was the most obvious.

2) TargetList - Why not use the built in linked list operations rather
than creating your own? Are they not accessible to client binaries
like this?

Overall I think this is a useful feature and I think you integrated it
well within the existing infrastructure, ie combining concepts of \set
and \g.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-16 Thread Shigeru HANADA
Hi Pavel,

On Tue, Oct 16, 2012 at 6:59 AM, Pavel Stehule pavel.steh...@gmail.com
wrote:
 here is updated patch, I moved lot of code from lexer to command.com,
 and now more \gset doesn't disable other backslash commands on same
 line.

* lexer changes
IIUC, new function psql_scan_varlist_varname scans input and returns a
variable name or a comma at each call, and command.c handles the error
such as invalid # of variables.  This new design seems better than old one.

However, IMHO the name psql_scan_varlist_varname sounds redundant and
unintuitive.  I'd prefer psql_scan_slash_varlist, because it indicates
that that function is expected to be used for arguments of backslash
commands, like psql_scan_slash_command and psql_scan_slash_option.
Thoughts?

* multiple meta command
Now both of the command sequences

  $ SELECT 1, 2 \gset var1, var2 \g foo.txt
  $ SELECT 1, 2 \g foo.txt \gset var1, var2

set var1 and v2 to 1 and 2 respectively, and also write the result
into foo.txt.  This would be what users expected.

* Duplication of variables
I found an issue we have not discussed.  Currently \gset accepts same
variable names in the list, and stores last SELECT item in duplicated
variables.  For instance,

  $ SELECT 1, 2 \gset var, var

stores 2 into var.  I think this behavior is acceptable, but it might
be worth mentioning in document.

* extra fixes
I fixed some minor issues below.  Please see attached v10 patch for details.

  * remove unused macro OT_VARLIST
  * remove unnecessary #include directive for common.h
  * fill comment within 80 columns
  * indent short variable name with tab
  * add regression test case for combination of \g and \gset

* bug on FETCH_COUNT = 1
When FETCH_COUNT is set to 1, and the number of rows returned is 1 too,
\gset shows extra (1 row).  This would be a bug in
ExecQueryUsingCursor.  Please see the last test case in regression test
psql_cmd.

I'll mark this patch as waiting author.

Regards,
-- 
Shigeru HANADA
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
***
*** 1483,1490  testdb=gt;
  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 or literal\g/ to send it, or
! literal\r/ to cancel.
  /para
  
  para
--- 1483,1490 
  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.
  /para
  
  para
***
*** 1617,1622  Tue Oct 26 21:40:57 CEST 1999
--- 1617,1644 
/varlistentry
  
varlistentry
+ termliteral\gset/literal replaceable class=parametervariable/replaceable [ ,replaceable class=parametervariable/replaceable ... ] /term
+ 
+ listitem
+ para
+  Sends the current query input buffer to the server and stores the
+  query's output into corresponding replaceable
+  class=parametervariable/replaceable.  The preceding query must
+  return only one row, and the number of variables must be same as the
+  number of elements in commandSELECT/command list.  If you don't
+  need any of items in commandSELECT/command list, you can omit
+  corresponding replaceable class=parametervariable/replaceable.
+  Example:
+ programlisting
+ foo=gt; SELECT 'hello', 'wonderful', 'world!' \gset var1,,var3 
+ foo=gt; \echo :var1 :var3
+ hello world!
+ /programlisting
+ /para
+ /listitem
+   /varlistentry
+ 
+   varlistentry
  termliteral\h/literal or literal\help/literal literal[ replaceable class=parametercommand/replaceable ]/literal/term
  listitem
  para
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***
*** 71,77  static void printSSLInfo(void);
  static void checkWin32Codepage(void);
  #endif
  
! 
  
  /*--
   * HandleSlashCmds:
--- 71,86 
  static void checkWin32Codepage(void);
  #endif
  
! /*
!  * Possible states for simple state machine, that is used for
!  * parsing target list - list of varnames separated by comma.
!  */
! typedef enum
! {
! 	VARLIST_INITIAL,
! 	VARLIST_EXPECTED_COMMA,
! 	VARLIST_EXPECTED_COMMA_OR_IDENT
! } VarlistParserState;
  
  /*--
   * HandleSlashCmds:
***
*** 748,753  exec_command(const char *cmd,
--- 757,831 
  		status = PSQL_CMD_SEND;
  	}
  
+ 	/* \gset send query and store result */
+ 	else if (strcmp(cmd, gset) == 0)
+ 	{
+ 		bool value_is_valid;
+ 		char *value;
+ 		VarlistParserState state = VARLIST_INITIAL;
+ 
+ 		/* expected valid target list */
+ 		success = true;
+ 
+ 		pset.gvars = NULL;
+ 		

Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-16 Thread Pavel Stehule
Hello

here is updated patch, I moved lot of code from lexer to command.com,
and now more \gset doesn't disable other backslash commands on same
line.

Regards

Pavel

2012/10/15 Pavel Stehule pavel.steh...@gmail.com:
 2012/10/15 Pavel Stehule pavel.steh...@gmail.com:
 2012/10/15 Shigeru HANADA shigeru.han...@gmail.com:
 Hi Pavel,

 First of all, I'm sorry that my previous review was rough.  I looked
 your patch and existing code closely again.

 On 2012/10/15, at 12:57, Pavel Stehule pavel.steh...@gmail.com wrote:
 2012/10/14 Tom Lane t...@sss.pgh.pa.us:
 * ExecQueryUsingCursor's concept of how to process command results
  (why? surely we don't need \gset to use a cursor)

 There was two possibilities, but hardly non using cursor is better way

 +1 for supporting the case when FETCH_COUNT  0, because user might set
 so mainly for other queries, and they would want to avoid changing
 FETCH_COUNT setting during every query followed by \gset.

 * the psql lexer (adding a whole bunch of stuff that probably doesn't
  belong there)

 ??

 I think that Tom is talking about psql_scan_slash_vars().  It seems too
 specific to \gset command.  How about to improve
 psql_scan_slash_options() so that it can handle comma-separated variable
 list?  Although you might have tried it before.
 # Unused OT_VARLIST macro gave me the idea.

 yes, it is possible - I'll look on it at evening

 a reuse of psql_scan_slash_options is not good idea - a interface of
 this function is out of my purposes - and I cannot to signalise error
 from this procedure. But I can minimize psql_scan_slash_var and I can
 move lot of code out of lexer file.



 * the core psql settings construct (to store something that is in
  no way a persistent setting)


 ??

 I thought that having \gset arguments in pset is reasonable, since \g
 uses pest.gfname to hold its one-shot setting.  Or, we should refactor
 \g to fit with \gset?  I might be missing Tom's point...

 Surely there is a less ugly and invasive way to do this.  The fact
 that the reviewer keeps finding bizarre bugs like another backslash
 command on the same line doesn't work seems to me to be a good
 indication that this is touching things it shouldn't.

 - all these bugs are based on lexer construct. A little modification
 of lexer is possible

 IMHO those issues come from the design rather than the implementation of
 lexer.  AFAIK we don't have consensus about the case that both of \g and
 \gset are used for a query like this:

 postgres=# SELECT 1 \gset var \\ \g foo.txt

 This seems regal.  Should we store 1 into var and write the result
 into foo.txt?  Or, only either of them?  It's just an idea and it
 requires new special character, but how about use \g command for file,
 pipe, and variable?  In the case we choose '' for variable prefix:

 postgres=# SELECT 'hello', 'wonderful', 'world!' \g var1,,var2

 Anyway, we've had no psql's meta command which processes query result
 other than \g.  So, we might have more considerable issues about design.

 a current design is rigid - a small implementation can stop parsing
 target list, when other backslash statement is detected


 BTW, what the word comma_expected means?  It's in the comment above
 psql_scan_slash_vars().  It might be a remaining of old implementation.

 This identifier is mistaken - etc this comment is wrong and related to
 old implementation - sorry. A first design was replaced by state
 machine described by  VarListParserState




 Regards,
 --
 Shigeru HANADA
 shigeru.han...@gmail.com







gset09.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-16 Thread Pavel Stehule
2012/10/16 Shigeru HANADA shigeru.han...@gmail.com:
 Hi Pavel,

 On Tue, Oct 16, 2012 at 6:59 AM, Pavel Stehule pavel.steh...@gmail.com
 wrote:
 here is updated patch, I moved lot of code from lexer to command.com,
 and now more \gset doesn't disable other backslash commands on same
 line.

 * lexer changes
 IIUC, new function psql_scan_varlist_varname scans input and returns a
 variable name or a comma at each call, and command.c handles the error
 such as invalid # of variables.  This new design seems better than old one.

 However, IMHO the name psql_scan_varlist_varname sounds redundant and
 unintuitive.  I'd prefer psql_scan_slash_varlist, because it indicates
 that that function is expected to be used for arguments of backslash
 commands, like psql_scan_slash_command and psql_scan_slash_option.
 Thoughts?

 * multiple meta command
 Now both of the command sequences

   $ SELECT 1, 2 \gset var1, var2 \g foo.txt
   $ SELECT 1, 2 \g foo.txt \gset var1, var2

 set var1 and v2 to 1 and 2 respectively, and also write the result
 into foo.txt.  This would be what users expected.

 * Duplication of variables
 I found an issue we have not discussed.  Currently \gset accepts same
 variable names in the list, and stores last SELECT item in duplicated
 variables.  For instance,

   $ SELECT 1, 2 \gset var, var

 stores 2 into var.  I think this behavior is acceptable, but it might
 be worth mentioning in document.

 * extra fixes
 I fixed some minor issues below.  Please see attached v10 patch for details.

   * remove unused macro OT_VARLIST
   * remove unnecessary #include directive for common.h
   * fill comment within 80 columns
   * indent short variable name with tab
   * add regression test case for combination of \g and \gset

 * bug on FETCH_COUNT = 1
 When FETCH_COUNT is set to 1, and the number of rows returned is 1 too,
 \gset shows extra (1 row).  This would be a bug in
 ExecQueryUsingCursor.  Please see the last test case in regression test
 psql_cmd.

I fixed this bug

Regards

Pavel


 I'll mark this patch as waiting author.

 Regards,
 --
 Shigeru HANADA


gset_11.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-15 Thread Pavel Stehule
Hello

2012/10/14 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 [ gset_08.diff ]

 In the course of adding a new backslash command, this patch manages to
 touch:

 * the main loop's concept of what types of backslash commands exist
   (PSQL_CMD_NOSEND ... what's the point of that, rather than making
   this work the same as \g?)

This is necessary, because there is a new possible state - query is
complete, but command is wrong - so I cannot use \g implementation,
because there is no possible error in \g or ';'

 * SendQuery's concept of how to process command results (again, why
   isn't this more like \g?)

it is similar - difference is only in work with result - \gset uses
StoreQueryResult instead PrintQueryResults, but other is same


 * ExecQueryUsingCursor's concept of how to process command results
   (why? surely we don't need \gset to use a cursor)

It is my mistake - simply and correct way is not using cursor in this use case

 * the psql lexer (adding a whole bunch of stuff that probably doesn't
   belong there)

I had to modify lexer - current lexer supports symbols separated by
space, but not symbols separated by comma. We don't would to use
variable evaluation in target variable list.

 * the core psql settings construct (to store something that is in
   no way a persistent setting)

sorry, I don't understand to this issue


 Surely there is a less ugly and invasive way to do this.  The fact
 that the reviewer keeps finding bizarre bugs like another backslash
 command on the same line doesn't work seems to me to be a good
 indication that this is touching things it shouldn't.


I had too strong in checking and raising errors. Is relative simple to
modify patch to enable more backslash statements on same line

I'll send updated patch early

Regards

Pavel

 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-15 Thread Pavel Stehule
2012/10/15 Pavel Stehule pavel.steh...@gmail.com:
 Hello

 2012/10/14 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 [ gset_08.diff ]

 In the course of adding a new backslash command, this patch manages to
 touch:

 * the main loop's concept of what types of backslash commands exist
   (PSQL_CMD_NOSEND ... what's the point of that, rather than making
   this work the same as \g?)

 This is necessary, because there is a new possible state - query is
 complete, but command is wrong - so I cannot use \g implementation,
 because there is no possible error in \g or ';'

 * SendQuery's concept of how to process command results (again, why
   isn't this more like \g?)

 it is similar - difference is only in work with result - \gset uses
 StoreQueryResult instead PrintQueryResults, but other is same


 * ExecQueryUsingCursor's concept of how to process command results
   (why? surely we don't need \gset to use a cursor)

 It is my mistake - simply and correct way is not using cursor in this use case

and it is question if cursor support is bad decision, because cursors
can help with large queries, that can returns thousands rows - and we
can raise error early, before we fall on out of memory

regards

Pavel


 * the psql lexer (adding a whole bunch of stuff that probably doesn't
   belong there)

 I had to modify lexer - current lexer supports symbols separated by
 space, but not symbols separated by comma. We don't would to use
 variable evaluation in target variable list.

 * the core psql settings construct (to store something that is in
   no way a persistent setting)

 sorry, I don't understand to this issue


 Surely there is a less ugly and invasive way to do this.  The fact
 that the reviewer keeps finding bizarre bugs like another backslash
 command on the same line doesn't work seems to me to be a good
 indication that this is touching things it shouldn't.


 I had too strong in checking and raising errors. Is relative simple to
 modify patch to enable more backslash statements on same line

 I'll send updated patch early

 Regards

 Pavel

 regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-15 Thread Pavel Stehule
2012/10/15 Shigeru HANADA shigeru.han...@gmail.com:
 Hi Pavel,

 First of all, I'm sorry that my previous review was rough.  I looked
 your patch and existing code closely again.

 On 2012/10/15, at 12:57, Pavel Stehule pavel.steh...@gmail.com wrote:
 2012/10/14 Tom Lane t...@sss.pgh.pa.us:
 * ExecQueryUsingCursor's concept of how to process command results
  (why? surely we don't need \gset to use a cursor)

 There was two possibilities, but hardly non using cursor is better way

 +1 for supporting the case when FETCH_COUNT  0, because user might set
 so mainly for other queries, and they would want to avoid changing
 FETCH_COUNT setting during every query followed by \gset.

 * the psql lexer (adding a whole bunch of stuff that probably doesn't
  belong there)

 ??

 I think that Tom is talking about psql_scan_slash_vars().  It seems too
 specific to \gset command.  How about to improve
 psql_scan_slash_options() so that it can handle comma-separated variable
 list?  Although you might have tried it before.
 # Unused OT_VARLIST macro gave me the idea.

yes, it is possible - I'll look on it at evening


 * the core psql settings construct (to store something that is in
  no way a persistent setting)


 ??

 I thought that having \gset arguments in pset is reasonable, since \g
 uses pest.gfname to hold its one-shot setting.  Or, we should refactor
 \g to fit with \gset?  I might be missing Tom's point...

 Surely there is a less ugly and invasive way to do this.  The fact
 that the reviewer keeps finding bizarre bugs like another backslash
 command on the same line doesn't work seems to me to be a good
 indication that this is touching things it shouldn't.

 - all these bugs are based on lexer construct. A little modification
 of lexer is possible

 IMHO those issues come from the design rather than the implementation of
 lexer.  AFAIK we don't have consensus about the case that both of \g and
 \gset are used for a query like this:

 postgres=# SELECT 1 \gset var \\ \g foo.txt

 This seems regal.  Should we store 1 into var and write the result
 into foo.txt?  Or, only either of them?  It's just an idea and it
 requires new special character, but how about use \g command for file,
 pipe, and variable?  In the case we choose '' for variable prefix:

 postgres=# SELECT 'hello', 'wonderful', 'world!' \g var1,,var2

 Anyway, we've had no psql's meta command which processes query result
 other than \g.  So, we might have more considerable issues about design.

a current design is rigid - a small implementation can stop parsing
target list, when other backslash statement is detected


 BTW, what the word comma_expected means?  It's in the comment above
 psql_scan_slash_vars().  It might be a remaining of old implementation.

This identifier is mistaken - etc this comment is wrong and related to
old implementation - sorry. A first design was replaced by state
machine described by  VarListParserState




 Regards,
 --
 Shigeru HANADA
 shigeru.han...@gmail.com







-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-15 Thread Pavel Stehule
2012/10/15 Pavel Stehule pavel.steh...@gmail.com:
 2012/10/15 Shigeru HANADA shigeru.han...@gmail.com:
 Hi Pavel,

 First of all, I'm sorry that my previous review was rough.  I looked
 your patch and existing code closely again.

 On 2012/10/15, at 12:57, Pavel Stehule pavel.steh...@gmail.com wrote:
 2012/10/14 Tom Lane t...@sss.pgh.pa.us:
 * ExecQueryUsingCursor's concept of how to process command results
  (why? surely we don't need \gset to use a cursor)

 There was two possibilities, but hardly non using cursor is better way

 +1 for supporting the case when FETCH_COUNT  0, because user might set
 so mainly for other queries, and they would want to avoid changing
 FETCH_COUNT setting during every query followed by \gset.

 * the psql lexer (adding a whole bunch of stuff that probably doesn't
  belong there)

 ??

 I think that Tom is talking about psql_scan_slash_vars().  It seems too
 specific to \gset command.  How about to improve
 psql_scan_slash_options() so that it can handle comma-separated variable
 list?  Although you might have tried it before.
 # Unused OT_VARLIST macro gave me the idea.

 yes, it is possible - I'll look on it at evening

a reuse of psql_scan_slash_options is not good idea - a interface of
this function is out of my purposes - and I cannot to signalise error
from this procedure. But I can minimize psql_scan_slash_var and I can
move lot of code out of lexer file.



 * the core psql settings construct (to store something that is in
  no way a persistent setting)


 ??

 I thought that having \gset arguments in pset is reasonable, since \g
 uses pest.gfname to hold its one-shot setting.  Or, we should refactor
 \g to fit with \gset?  I might be missing Tom's point...

 Surely there is a less ugly and invasive way to do this.  The fact
 that the reviewer keeps finding bizarre bugs like another backslash
 command on the same line doesn't work seems to me to be a good
 indication that this is touching things it shouldn't.

 - all these bugs are based on lexer construct. A little modification
 of lexer is possible

 IMHO those issues come from the design rather than the implementation of
 lexer.  AFAIK we don't have consensus about the case that both of \g and
 \gset are used for a query like this:

 postgres=# SELECT 1 \gset var \\ \g foo.txt

 This seems regal.  Should we store 1 into var and write the result
 into foo.txt?  Or, only either of them?  It's just an idea and it
 requires new special character, but how about use \g command for file,
 pipe, and variable?  In the case we choose '' for variable prefix:

 postgres=# SELECT 'hello', 'wonderful', 'world!' \g var1,,var2

 Anyway, we've had no psql's meta command which processes query result
 other than \g.  So, we might have more considerable issues about design.

 a current design is rigid - a small implementation can stop parsing
 target list, when other backslash statement is detected


 BTW, what the word comma_expected means?  It's in the comment above
 psql_scan_slash_vars().  It might be a remaining of old implementation.

 This identifier is mistaken - etc this comment is wrong and related to
 old implementation - sorry. A first design was replaced by state
 machine described by  VarListParserState




 Regards,
 --
 Shigeru HANADA
 shigeru.han...@gmail.com







-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-14 Thread Pavel Stehule
Hello

2012/10/14 Erik Rijkers e...@xs4all.nl:
 On Sat, October 13, 2012 19:26, Pavel Stehule wrote:
 2012/10/13 Shigeru HANADA shigeru.han...@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



 Thanks

 Erik Rijkers



gset_08.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-14 Thread Shigeru HANADA

Hi Pavel,

On Sat, Oct 13, 2012 at 12:58 AM, Pavel Stehule 
pavel.steh...@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
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
***
*** 1483,1490  testdb=gt;
  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 or literal\g/ to send it, or
! literal\r/ to cancel.
  /para
  
  para
--- 1483,1490 
  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.
  /para
  
  para
***
*** 1617,1622  Tue Oct 26 21:40:57 CEST 1999
--- 1617,1644 
/varlistentry
  
varlistentry
+ termliteral\gset/literal replaceable 
class=parametervariable/replaceable [ ,replaceable 
class=parametervariable/replaceable ... ] /term
+ 
+ listitem
+ para
+  Sends the current query input buffer to the server and stores the
+  query's output into corresponding replaceable
+  class=parametervariable/replaceable.  The preceding query must
+  return only one row, and the number of variables must be same as the
+  number of elements in commandSELECT/command list.  If you don't
+  need any of items in commandSELECT/command list, you can omit
+  corresponding replaceable class=parametervariable/replaceable.
+  Example:
+ programlisting
+ foo=gt; SELECT 'hello', 'wonderful', 'world!' \gset var1,,var3 
+ foo=gt; \echo :var1 :var3
+ hello world!
+ /programlisting
+ /para
+ /listitem
+   /varlistentry
+ 
+   varlistentry
  termliteral\h/literal or literal\help/literal literal[ 
replaceable class=parametercommand/replaceable ]/literal/term
  listitem
  para
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***
*** 748,753  exec_command(const char *cmd,
--- 748,776 
status = PSQL_CMD_SEND;
}
  
+   /* \gset send query and store result */
+   else if (strcmp(cmd, gset) == 0)
+   {
+   boolerror;
+ 
+   pset.gvars = psql_scan_slash_vars(scan_state, error);
+ 
+   if (!pset.gvars  !error)
+   {
+   psql_error(\\%s: missing required argument\n, cmd);
+   status = PSQL_CMD_NOSEND;
+   }
+   else if (error)
+   {
+   psql_error(\\%s: syntax error\n, cmd);
+   status = PSQL_CMD_NOSEND;
+   tglist_free(pset.gvars);
+   pset.gvars = NULL;
+   }
+   else
+   status = PSQL_CMD_SEND;
+   }
+ 
/* help */
else if (strcmp(cmd, h) == 0 || strcmp(cmd, help) == 0)
{
*** a/src/bin/psql/command.h
--- b/src/bin/psql/command.h
***
*** 16,21  typedef enum _backslashResult
--- 16,22 
  {
PSQL_CMD_UNKNOWN = 0,   /* not done parsing yet (internal only) 
*/
PSQL_CMD_SEND,  /* query complete; send off */
+   PSQL_CMD_NOSEND,/* query complete, don't send */
PSQL_CMD_SKIP_LINE, /* keep building query */
PSQL_CMD_TERMINATE, /* quit program */
PSQL_CMD_NEWEDIT,   /* query buffer was changed 
(e.g., via \e) */
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
***
*** 816,821  PrintQueryResults(PGresult *results)
--- 816,919 
return success;
  }
  
+ /*
+  * StoreQueryResult: store first row of result to selected variables
+  *
+  * Note: Utility function for use by SendQuery() only.
+  *
+  * Returns true if the query executed successfully, false otherwise.
+  */
+ static bool
+ StoreQueryResult(PGresult *result)
+ {
+   bool success;
+ 
+   switch (PQresultStatus(result))
+   {
+   case PGRES_TUPLES_OK:
+   {
+   int i;
+ 
+   if (PQntuples(result)  1)
+ 

Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-13 Thread Pavel Stehule
2012/10/13 Shigeru HANADA shigeru.han...@gmail.com:
 Hi Pavel,


 On Sat, Oct 13, 2012 at 12:58 AM, Pavel Stehule pavel.steh...@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


 Regards,
 --
 Shigeru HANADA


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-13 Thread Erik Rijkers
On Sat, October 13, 2012 19:26, Pavel Stehule wrote:
 2012/10/13 Shigeru HANADA shigeru.han...@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



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-10-12 Thread Pavel Stehule
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.steh...@gmail.com:
 Hello

 2012/9/21 Shigeru HANADA shigeru.han...@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


gset_06.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-09-27 Thread Pavel Stehule
Hello

2012/9/21 Shigeru HANADA shigeru.han...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-09-22 Thread Pavel Stehule
Hello

2012/9/19 Shigeru HANADA shigeru.han...@gmail.com:
 On Fri, Aug 10, 2012 at 3:21 AM, Pavel Stehule pavel.steh...@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


gset_04.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-09-22 Thread Shigeru HANADA
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=gt;
 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=parametervariable/replaceable.  The preceding query must
+ return only one row, and the number of variables must be same as the
+ number of elements in commandSELECT/command list.  If you don't
+ need any of items in commandSELECT/command list, you can omit
+ corresponding replaceable class=parametervariable/replaceable.
+ Example:
+programlisting
+foo=gt; SELECT 'hello', 'wonderful', 'world!' \gset var1,,var3 
+foo=gt; \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


Re: [HACKERS] proposal - assign result of query to psql variable

2012-09-19 Thread Shigeru HANADA
On Fri, Aug 10, 2012 at 3:21 AM, Pavel Stehule pavel.steh...@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
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 0e9b408..a76b84d 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -832,7 +832,7 @@ PrintQueryResults(PGresult *results)
  *
  * Note: Utility function for use by SendQuery() only.
  *
- * Returns true if the query executed sucessfully, false otherwise.
+ * Returns true if the query executed successfully, false otherwise.
  */
 static bool
 StoreQueryResult(PGresult *result)
@@ -865,7 +865,7 @@ StoreQueryResult(PGresult *result)
{
if (!iter)
{
-   psql_error(to few 
target variables\n);
+   psql_error(too few 
target variables\n);
success = false;
break;
}
@@ -902,7 +902,7 @@ StoreQueryResult(PGresult *result)
 
case PGRES_COPY_OUT:
case PGRES_COPY_IN:
-   psql_error(COPY isnot supported by \\gset command\n);
+   psql_error(COPY is not supported by \\gset command\n);
success = false;
break;
 
@@ -1797,7 +1797,7 @@ expand_tilde(char **filename)
 
 
 /*
- * Add name of internal variable to query targer list
+ * Add name of internal variable to query target list
  *
  */
 TargetList
diff --git a/src/test/regress/expected/psql.out 
b/src/test/regress/expected/psql.out
index 90ab9bd..54fa490 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -15,7 +15,7 @@ select 10, 20, 'Hello World'
 -- should fail
 \gset ,,
 \gset ,
-to few target variables
+too few target variables
 \gset ,,,
 too many target variables
 -- should be ok

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-08-09 Thread Pavel Stehule
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 da...@fetter.org:
 On Sat, Jul 28, 2012 at 06:11:21PM +0200, Pavel Stehule wrote:
 Hello

 2012/7/27 Tom Lane t...@sss.pgh.pa.us:
  Pavel Stehule pavel.steh...@gmail.com writes:
  2012/7/26 David Fetter da...@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 da...@fetter.org http://fetter.org/
 Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
 Skype: davidfetter  XMPP: david.fet...@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


gset_03.diff
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-31 Thread David Fetter
On Sat, Jul 28, 2012 at 06:11:21PM +0200, Pavel Stehule wrote:
 Hello
 
 2012/7/27 Tom Lane t...@sss.pgh.pa.us:
  Pavel Stehule pavel.steh...@gmail.com writes:
  2012/7/26 David Fetter da...@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 da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@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
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
***
*** 1489,1495  testdb=gt;
  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 or literal\g/ to send it, or
  literal\r/ to cancel.
  /para
  
--- 1489,1495 
  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.
  /para
  
***
*** 1623,1628  Tue Oct 26 21:40:57 CEST 1999
--- 1623,1640 
/varlistentry
  
varlistentry
+ termliteral\gset/literal replaceable 
class=parametervariable/replaceable [ ,replaceable 
class=parametervariable/replaceable ... ] /term
+ 
+ listitem
+ para
+ Sends the current query input buffer to the server and stores
+ the query's target list a corresponding list of psql
+ variables.
+ /para
+ /listitem
+   /varlistentry
+ 
+   varlistentry
  termliteral\h/literal or literal\help/literal literal[ 
replaceable class=parametercommand/replaceable ]/literal/term
  listitem
  para
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***
*** 748,753  exec_command(const char *cmd,
--- 748,776 
status = PSQL_CMD_SEND;
}
  
+   /* \gset send query and store result */
+   else if (strcmp(cmd, gset) == 0)
+   {
+   boolerror;
+ 
+   pset.gvars = psql_scan_slash_vars(scan_state, error);
+ 
+   if (!pset.gvars)
+   {
+   psql_error(\\%s: missing required argument\n, cmd);
+   status = PSQL_CMD_NOSEND;
+   }
+   else if (error)
+   {
+   psql_error(\\%s: syntax error\n, cmd);
+   status = PSQL_CMD_NOSEND;
+   tglist_free(pset.gvars);
+   pset.gvars = NULL;
+   }
+   else
+   status = PSQL_CMD_SEND;
+   }
+ 
/* help */
else if (strcmp(cmd, h) == 0 || strcmp(cmd, help) == 0)
{
*** a/src/bin/psql/command.h
--- b/src/bin/psql/command.h
***
*** 16,21  typedef enum _backslashResult
--- 16,22 
  {
PSQL_CMD_UNKNOWN = 0,   /* not done parsing yet (internal only) 
*/
PSQL_CMD_SEND,  /* query complete; send off */
+   PSQL_CMD_NOSEND,/* query complete, don't send */
PSQL_CMD_SKIP_LINE, /* keep building query */
PSQL_CMD_TERMINATE, /* quit program */
PSQL_CMD_NEWEDIT,   /* query buffer was changed 
(e.g., via \e) */
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
***
*** 826,831  PrintQueryResults(PGresult *results)

Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-28 Thread Pavel Stehule
Hello

2012/7/27 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@gmail.com writes:
 2012/7/26 David Fetter da...@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






 regards, tom lane


gset.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-26 Thread David Fetter
On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:
 Pavel Stehule pavel.steh...@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 da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-26 Thread Pavel Stehule
2012/7/26 David Fetter da...@fetter.org:
 On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:
 Pavel Stehule pavel.steh...@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


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

 Cheers,
 David.
 --
 David Fetter da...@fetter.org http://fetter.org/
 Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
 Skype: davidfetter  XMPP: david.fet...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-26 Thread David Fetter
On Thu, Jul 26, 2012 at 08:31:13AM +0200, Pavel Stehule wrote:
 2012/7/26 David Fetter da...@fetter.org:
  On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:
  Pavel Stehule pavel.steh...@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 da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-26 Thread Pavel Stehule
2012/7/26 David Fetter da...@fetter.org:
 On Thu, Jul 26, 2012 at 08:31:13AM +0200, Pavel Stehule wrote:
 2012/7/26 David Fetter da...@fetter.org:
  On Thu, Jul 26, 2012 at 01:36:17AM -0400, Tom Lane wrote:
  Pavel Stehule pavel.steh...@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



 Cheers,
 David.

 P.S.  The bike shed should be puce with blaze orange pin-striping.
 --
 David Fetter da...@fetter.org http://fetter.org/
 Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
 Skype: davidfetter  XMPP: david.fet...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-26 Thread Tom Lane
Pavel Stehule pavel.steh...@gmail.com writes:
 2012/7/26 David Fetter da...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-25 Thread Tom Lane
Pavel Stehule pavel.steh...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal - assign result of query to psql variable

2012-07-25 Thread Pavel Stehule
2012/7/26 Tom Lane t...@sss.pgh.pa.us:
 Pavel Stehule pavel.steh...@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


 regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers