Re: [HACKERS] Review: query result history in psql

2013-07-05 Thread Josh Berkus
On 07/02/2013 06:16 PM, Robert Haas wrote:
 I'm kinda not all that convinced that this feature does anything
 that's actually useful.  If you want to save your query results, you
 can just stick CREATE TEMP TABLE ans AS in front of your query.
 What does that not give you that this gives you?

Convenience.  Think of the times when you were doing a quick check on
some query result from several different queries and wanted to flip back
and forth between them, but you can't do that by scrolling because the
pager has the query result and hides it from terminal scrolling.  If we
had this feature, I'd use it a lot, personally.

My take on the issues discussed:

Interactive Mode: I personally don't see value in this, and wouldn't use
it if it existed.  Plus even if there is value in it, it could be added
later on, so shouldn't block this patch.

Finding Query Results: I don't find the approach of ans01/ans02/ans03
useful.  For any place where I really need this feature, I'm going to
have enough buffered queries that there's no way I can remember which is
which.  I don't, however, have an immediate solution for something which
would be overall easier.  Maybe a prompt after each query for what name
to put it in the history as?  Not sure.

ans: I agree that this is not intuitive for most DBAs.  The other
applications which use that abbreviation do not have sufficient overlap
with PostgreSQL for it to be natural.  What about just result?

Size Limits: before this becomes a PostgreSQL feature, we really do need
to have some limits, both for total memory size, and for number of saved
query result sets.  Otherwise we'll have lots of people crashing their
clients because they forgot that result history was on.

Also, I'd like to think some about how this could, potentially, in the
future tie in to being able to dispatch asyncronous queries from psql.
If we have a query result cache, it's one short step to allowing that
result cache to be populated asyncrhonously.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Review: query result history in psql

2013-07-02 Thread Ian Link
Maciej - I can see your 
resistance to some kind of interactive mode. It would definitely be more
 code and create a less simple user interface. I would be perfectly 
happy if we left that part as it is now. 

However, I think it would be important to have a way of displaying the 
query history. Yes, you can search through your console backwards. You 
can still search through your old queries, but this would be a good 
alternative. What if the user reconnects after working for a while? 
Their old bash history might be gone. This would leave them with a big 
query history and no point of reference. Personally, I would find this 
feature very worthwhile. The query history wouldn't be crippled without 
it, but it would be a lot less flexible.


   	   
   	Pavel Stehule  
  Monday, July 01, 
2013 4:05 AM
  a idea is good,
 but I don't think, it can be useful with currentimplementation. How
 I can identify, what is correct answer for myquery? Have I remember
 twenty numbers and twenty queries?RegardsPavel
   	   
   	Maciej Gajewski  
  Monday, July 01, 
2013 4:01 AM
  When I tested this feature, I had 30 caches per 5 
minutes, and only a

few from these queries had a sense. Switch between off and on is not
user friendly. I believe so there can be other solution than mine, but
a possibility to friendly clean unwanted caches is necessary. If you know that you'll need the result of a query 
beforehand, you can always use SELECT ... INTO ... . No client-side 
features required.
This feature is intended for 
people running plenty of ad-hoc queries, when every result could 
potentially be useful.

  
   	   
   	Pavel Stehule  
  Monday, July 01, 
2013 1:31 AM
  2013/7/1 Maciej Gajewski maciej.gajews...@gmail.com:
I'm not really bought into some of the ideas.


but maybe some interactive mode should be usefull - so after
execution, and showing result, will be prompt if result should be
saved or not.
I like the idea, in addition to the ordinary mode. Personally, I would use
the ordinary mode, but I can see how 'interactive' would be useful.

This would require a complex change to the client code. And the result would
eventually become annoying: an interactive question after each and every
query. Currently, when turned on, every result is stored and simple
notification is printed.

.
When I tested this feature, I had 30 caches per 5 minutes, and only a
few from these queries had a sense. Switch between off and on is not
user friendly. I believe so there can be other solution than mine, but
a possibility to friendly clean unwanted caches is necessary.

 yes, the names :ans01, :ans02, ... miss semantics - How I can join

this name (and content) with some SQL query?
That makes sense. I think having part of / the whole query string would be
very helpful. Great suggestion!

The naming is obscure and non-informative, I agree. If you have a nice idea
how to make it better, I'd love to discuss it. But please remember that it
has one huge advantage: simplicity. The client is a classical command-line
tool, and as such it delegates some of the functionality to external
programs, like pager or terminal.


Personally, I don't see a strong price for all users without friendly
interface.

Regards

Pavel

I'm pretty sure that your terminal emulator has a 'find' function that would
allow you to quickly locate the variable and associated query in the
scrollback.

M


   	   
   	Maciej Gajewski  
  Monday, July 01, 
2013 1:23 AM
  I'm not really bought into some of the ideas.
but maybe some 
interactive mode should be usefull - so after

execution, and
 showing result, will be prompt if result should besaved or not.

I like the idea, in addition to the ordinary mode. 
Personally, I would use the ordinary mode, but I can see how 
'interactive' would be useful. 
This would require a complex change to the 
client code. And the result would eventually become annoying: an 
interactive question after each and every query. Currently, when turned 
on, every result is stored and simple notification is printed.
 

 yes, the 
names :ans01, :ans02, ... miss semantics - How I can join

this name (and
 content) with some SQL query?That makes 
sense. I think having part of / the whole query string would be very 
helpful. Great suggestion! 

The naming is obscure and non-informative, I agree. If you have
 a nice idea how to make it better, I'd love to discuss it. But please 
remember that it has one huge advantage: simplicity. The client is a 
classical command-line tool, and as such it delegates some of the 
functionality to external programs, like pager or terminal.
I'm pretty sure that your terminal
 emulator has a 'find' function that would allow you to quickly locate 
the variable and associated query in the scrollback.
M

  
   	   
   	ian link  
  Monday, July 01, 
2013 12:19 AM
  but maybe some 
interactive mode should be usefull - so after
execution, and
 showing result, will be prompt if result should besaved or not.
I 

Re: [HACKERS] Review: query result history in psql

2013-07-02 Thread Maciej Gajewski
The query history is stored within the client, so once the user stops
the client, it is gone. But yes, it would be useful to have some tool
that would allow you to see what's in there.

I could be a command (\showans ?) that would list all :ansXXX
variables, together with the query text and  the size of the answer.
It would probably look ugly for very long queries, but could be useful
anyway.

I'm not sure if I'll be able to implement this feature any time soon,
as I'm very busy at the moment and going for a business trip in few
days.

In the meantime, I've applied your suggestions and moved the
sting-manipulating functions to stringutils. Also fixed a tiny bug.
Patch attached.

M


psql-ans.3.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] Review: query result history in psql

2013-07-02 Thread Peter Eisentraut
On 7/2/13 5:53 AM, Maciej Gajewski wrote:
 In the meantime, I've applied your suggestions and moved the
 sting-manipulating functions to stringutils. Also fixed a tiny bug.
 Patch attached.

I haven't been able to find any real documentation on this feature,
other than the additions to the psql help.  Could someone write some
mock documentation at least, so we know what the proposed interfaces and
intended ways of use are?

What is ans?



-- 
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] Review: query result history in psql

2013-07-02 Thread ian link

 I haven't been able to find any real documentation on this feature,
 other than the additions to the psql help.

You're right, I missed that in my review. I agree that it needs some more
documentation.

What is ans?

We covered that earlier in the email thread, but it's the current name for
the query history. I think we are going to change it to something a little
more descriptive. I was thinking qh for short or query-history.

I'm not sure if I'll be able to implement this feature any time soon,
 as I'm very busy at the moment and going for a business trip in few
 days.

I can try implementing the feature, I might have some time.




On Tue, Jul 2, 2013 at 5:36 AM, Peter Eisentraut pete...@gmx.net wrote:

 On 7/2/13 5:53 AM, Maciej Gajewski wrote:
  In the meantime, I've applied your suggestions and moved the
  sting-manipulating functions to stringutils. Also fixed a tiny bug.
  Patch attached.

 I haven't been able to find any real documentation on this feature,
 other than the additions to the psql help.  Could someone write some
 mock documentation at least, so we know what the proposed interfaces and
 intended ways of use are?

 What is ans?




Re: [HACKERS] Review: query result history in psql

2013-07-02 Thread Robert Haas
On Tue, Jul 2, 2013 at 8:16 PM, ian link i...@ilink.io wrote:
 We covered that earlier in the email thread, but it's the current name for
 the query history. I think we are going to change it to something a little
 more descriptive. I was thinking qh for short or query-history.

 I'm not sure if I'll be able to implement this feature any time soon,
 as I'm very busy at the moment and going for a business trip in few
 days.

 I can try implementing the feature, I might have some time.

I'm kinda not all that convinced that this feature does anything
that's actually useful.  If you want to save your query results, you
can just stick CREATE TEMP TABLE ans AS in front of your query.
What does that not give you that this gives you?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] Review: query result history in psql

2013-07-02 Thread ian link

 I'm kinda not all that convinced that this feature does anything
 that's actually useful.  If you want to save your query results, you
 can just stick CREATE TEMP TABLE ans AS in front of your query.
 What does that not give you that this gives you?

Convenience. It auto-increments with each new query, giving you a different
temporary table for each query. Seems pretty helpful to me.


On Tue, Jul 2, 2013 at 6:16 PM, Robert Haas robertmh...@gmail.com wrote:

 On Tue, Jul 2, 2013 at 8:16 PM, ian link i...@ilink.io wrote:
  We covered that earlier in the email thread, but it's the current name
 for
  the query history. I think we are going to change it to something a
 little
  more descriptive. I was thinking qh for short or query-history.
 
  I'm not sure if I'll be able to implement this feature any time soon,
  as I'm very busy at the moment and going for a business trip in few
  days.
 
  I can try implementing the feature, I might have some time.

 I'm kinda not all that convinced that this feature does anything
 that's actually useful.  If you want to save your query results, you
 can just stick CREATE TEMP TABLE ans AS in front of your query.
 What does that not give you that this gives you?

 --
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company



Re: [HACKERS] Review: query result history in psql

2013-07-02 Thread Pavel Stehule
2013/7/3 Robert Haas robertmh...@gmail.com:
 On Tue, Jul 2, 2013 at 8:16 PM, ian link i...@ilink.io wrote:
 We covered that earlier in the email thread, but it's the current name for
 the query history. I think we are going to change it to something a little
 more descriptive. I was thinking qh for short or query-history.

 I'm not sure if I'll be able to implement this feature any time soon,
 as I'm very busy at the moment and going for a business trip in few
 days.

 I can try implementing the feature, I might have some time.

 I'm kinda not all that convinced that this feature does anything
 that's actually useful.  If you want to save your query results, you
 can just stick CREATE TEMP TABLE ans AS in front of your query.
 What does that not give you that this gives you?

it solve lot of chars. I am thinking so idea is interesting.

I can imagine, so it can works similar like our \g statement, but
result will be stored to temp table and immediately showed.

Regards

Pavel


 --
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company


-- 
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] Review: query result history in psql

2013-07-01 Thread Pavel Stehule
Hello

2013/7/1 ian link i...@ilink.io:
 Not sure about all of your suggestions. Let me see if I can clarify what
 you're looking for.


  * simply decision if content should be stored in history or not,

 Do you mean that the user should use a flag to place the result of a query
 into the history?
 like:
 --ans SELECT * FROM cities...
 Not sure if that's what you mean, but it seems kind of unnecesary. They can
 just hit the \ans flag beforehand.

switching off on is not user friendly

but maybe some interactive mode should be usefull - so after
execution, and showing result, will be prompt if result should be
saved or not.

some like:

\ans interactive
 SELECT * FROM pg_proc;

 result 

should be saved last result [y, n]?
 y
result is saved in :ans22





 * simply remove last entry (table) of history

 That could be useful. What do you think Maciej?

yes, lot of queries is just +/- experiment and you don't would store result


  * queries should be joined to content, only name is not enough

 Don't know what you mean. Could you try re-wording that?


yes, the names :ans01, :ans02, ... miss semantics - How I can join
this name (and content) with some SQL query?

I needs to reverese search in SQL of stored caches, and I need a information

ans01  SELECT * FROM pg_proc
ans02  SELECT * FROM ans02 WHERE ...
ans03 ...

Regards

Pavel

 Ian



 On Fri, Jun 28, 2013 at 8:49 AM, Pavel Stehule pavel.steh...@gmail.com
 wrote:

 Hello

 I am not sure, this interface is really user friendly

 there is not possible searching in history, and not every query push
 to history some interesting content.

 It require:

 * simply decision if content should be stored in history or not,
 * simply remove last entry (table) of history
 * queries should be joined to content, only name is not enough

 Regards

 Pavel

 2013/6/28 Maciej Gajewski maciej.gajews...@gmail.com:
  Thanks for checking the patch!
 
  So what's left to fix?
  * Moving the escaping-related functions to separate module,
  * applying your corrections.
 
  Did I missed anything?
 
  I'll submit corrected patch after the weekend.
 
  M
 




-- 
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] Review: query result history in psql

2013-07-01 Thread ian link

 but maybe some interactive mode should be usefull - so after
 execution, and showing result, will be prompt if result should be
 saved or not.

I like the idea, in addition to the ordinary mode. Personally, I would use
the ordinary mode, but I can see how 'interactive' would be useful.

 yes, the names :ans01, :ans02, ... miss semantics - How I can join

this name (and content) with some SQL query?

That makes sense. I think having part of / the whole query string would be
very helpful. Great suggestion!

Maciej, would you be able/have time to implement these? Or do you need any
help getting them done?



On Sun, Jun 30, 2013 at 11:35 PM, Pavel Stehule pavel.steh...@gmail.comwrote:

 Hello

 2013/7/1 ian link i...@ilink.io:
  Not sure about all of your suggestions. Let me see if I can clarify what
  you're looking for.
 
 
   * simply decision if content should be stored in history or not,
 
  Do you mean that the user should use a flag to place the result of a
 query
  into the history?
  like:
  --ans SELECT * FROM cities...
  Not sure if that's what you mean, but it seems kind of unnecesary. They
 can
  just hit the \ans flag beforehand.

 switching off on is not user friendly

 but maybe some interactive mode should be usefull - so after
 execution, and showing result, will be prompt if result should be
 saved or not.

 some like:

 \ans interactive
  SELECT * FROM pg_proc;

  result 

 should be saved last result [y, n]?
  y
 result is saved in :ans22

 


 
  * simply remove last entry (table) of history
 
  That could be useful. What do you think Maciej?

 yes, lot of queries is just +/- experiment and you don't would store result

 
   * queries should be joined to content, only name is not enough
 
  Don't know what you mean. Could you try re-wording that?
 

 yes, the names :ans01, :ans02, ... miss semantics - How I can join
 this name (and content) with some SQL query?

 I needs to reverese search in SQL of stored caches, and I need a
 information

 ans01  SELECT * FROM pg_proc
 ans02  SELECT * FROM ans02 WHERE ...
 ans03 ...

 Regards

 Pavel

  Ian
 
 
 
  On Fri, Jun 28, 2013 at 8:49 AM, Pavel Stehule pavel.steh...@gmail.com
  wrote:
 
  Hello
 
  I am not sure, this interface is really user friendly
 
  there is not possible searching in history, and not every query push
  to history some interesting content.
 
  It require:
 
  * simply decision if content should be stored in history or not,
  * simply remove last entry (table) of history
  * queries should be joined to content, only name is not enough
 
  Regards
 
  Pavel
 
  2013/6/28 Maciej Gajewski maciej.gajews...@gmail.com:
   Thanks for checking the patch!
  
   So what's left to fix?
   * Moving the escaping-related functions to separate module,
   * applying your corrections.
  
   Did I missed anything?
  
   I'll submit corrected patch after the weekend.
  
   M
  
 
 



Re: [HACKERS] Review: query result history in psql

2013-07-01 Thread Maciej Gajewski
I'm not really bought into some of the ideas.


but maybe some interactive mode should be usefull - so after
 execution, and showing result, will be prompt if result should be
 saved or not.

 I like the idea, in addition to the ordinary mode. Personally, I would use
 the ordinary mode, but I can see how 'interactive' would be useful.


This would require a complex change to the client code. And the result
would eventually become annoying: an interactive question after each and
every query. Currently, when turned on, every result is stored and simple
notification is printed.



  yes, the names :ans01, :ans02, ... miss semantics - How I can join

 this name (and content) with some SQL query?

 That makes sense. I think having part of / the whole query string would be
 very helpful. Great suggestion!



The naming is obscure and non-informative, I agree. If you have a nice idea
how to make it better, I'd love to discuss it. But please remember that it
has one huge advantage: simplicity. The client is a classical command-line
tool, and as such it delegates some of the functionality to external
programs, like pager or terminal.

I'm pretty sure that your terminal emulator has a 'find' function that
would allow you to quickly locate the variable and associated query in the
scrollback.

M


Re: [HACKERS] Review: query result history in psql

2013-07-01 Thread Pavel Stehule
2013/7/1 Maciej Gajewski maciej.gajews...@gmail.com:
 I'm not really bought into some of the ideas.


 but maybe some interactive mode should be usefull - so after
 execution, and showing result, will be prompt if result should be
 saved or not.

 I like the idea, in addition to the ordinary mode. Personally, I would use
 the ordinary mode, but I can see how 'interactive' would be useful.


 This would require a complex change to the client code. And the result would
 eventually become annoying: an interactive question after each and every
 query. Currently, when turned on, every result is stored and simple
 notification is printed.

.
When I tested this feature, I had 30 caches per 5 minutes, and only a
few from these queries had a sense. Switch between off and on is not
user friendly. I believe so there can be other solution than mine, but
a possibility to friendly clean unwanted caches is necessary.



  yes, the names :ans01, :ans02, ... miss semantics - How I can join

 this name (and content) with some SQL query?

 That makes sense. I think having part of / the whole query string would be
 very helpful. Great suggestion!



 The naming is obscure and non-informative, I agree. If you have a nice idea
 how to make it better, I'd love to discuss it. But please remember that it
 has one huge advantage: simplicity. The client is a classical command-line
 tool, and as such it delegates some of the functionality to external
 programs, like pager or terminal.


Personally, I don't see a strong price for all users without friendly
interface.

Regards

Pavel

 I'm pretty sure that your terminal emulator has a 'find' function that would
 allow you to quickly locate the variable and associated query in the
 scrollback.

 M



-- 
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] Review: query result history in psql

2013-07-01 Thread Maciej Gajewski
 When I tested this feature, I had 30 caches per 5 minutes, and only a
 few from these queries had a sense. Switch between off and on is not
 user friendly. I believe so there can be other solution than mine, but
 a possibility to friendly clean unwanted caches is necessary.


 If you know that you'll need the result of a query beforehand, you can
always use SELECT ... INTO ... . No client-side features required.

This feature is intended for people running plenty of ad-hoc queries, when
every result could potentially be useful.


Re: [HACKERS] Review: query result history in psql

2013-07-01 Thread Pavel Stehule
2013/7/1 Maciej Gajewski maciej.gajews...@gmail.com:

 When I tested this feature, I had 30 caches per 5 minutes, and only a
 few from these queries had a sense. Switch between off and on is not
 user friendly. I believe so there can be other solution than mine, but
 a possibility to friendly clean unwanted caches is necessary.


  If you know that you'll need the result of a query beforehand, you can
 always use SELECT ... INTO ... . No client-side features required.

 This feature is intended for people running plenty of ad-hoc queries, when
 every result could potentially be useful.


a idea is good, but I don't think, it can be useful with current
implementation. How I can identify, what is correct answer for my
query? Have I remember twenty numbers and twenty queries?

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] Review: query result history in psql

2013-06-30 Thread ian link
Not sure about all of your suggestions. Let me see if I can clarify what
you're looking for.


  * simply decision if content should be stored in history or not,

Do you mean that the user should use a flag to place the result of a query
into the history?
like:
--ans SELECT * FROM cities...
Not sure if that's what you mean, but it seems kind of unnecesary. They can
just hit the \ans flag beforehand.

* simply remove last entry (table) of history

That could be useful. What do you think Maciej?

 * queries should be joined to content, only name is not enough

Don't know what you mean. Could you try re-wording that?

Ian



On Fri, Jun 28, 2013 at 8:49 AM, Pavel Stehule pavel.steh...@gmail.comwrote:

 Hello

 I am not sure, this interface is really user friendly

 there is not possible searching in history, and not every query push
 to history some interesting content.

 It require:

 * simply decision if content should be stored in history or not,
 * simply remove last entry (table) of history
 * queries should be joined to content, only name is not enough

 Regards

 Pavel

 2013/6/28 Maciej Gajewski maciej.gajews...@gmail.com:
  Thanks for checking the patch!
 
  So what's left to fix?
  * Moving the escaping-related functions to separate module,
  * applying your corrections.
 
  Did I missed anything?
 
  I'll submit corrected patch after the weekend.
 
  M
 



Re: [HACKERS] Review: query result history in psql

2013-06-28 Thread ian link

 It's better to post a review as a reply to the message which contains
 the patch.

Sorry about that, I did not have the email in my inbox and couldn't figure
out how to use the old message ID to send a reply. Here is the thread:
http://www.postgresql.org/message-id/flat/caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com#caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com

The 'EscapeForCopy' was meant to mean 'Escape string in a format require by
 the COPY TEXT format', so 'copy' in the name refers to the escaping format,
 not the action performed by the function.


I see, that makes sense now. Keep it as you see fit, it's not a big deal in
my opinion.

 Some mathematical toolkits, like Matlab or Mathematica, automatically set
 a variable called 'ans' (short for answer) containing the result of the
 last operation. I was trying to emulate exactly this behaviour.


I've actually been using Matlab lately, which must be why the name made
sense to me intuitively. I don't know if this is the best name, however. It
kind of assumes that our users use Matlab/Octave/Mathematica. Maybe 'qhist'
or 'hist' or something?

The history is not erased. The history is always stored in the client's
 memory.

Ah, I did not pick up on that. Thank you for explaining it! That's actually
a very neat way of doing it. Sorry I did not realize that at first.

I was considering such a behaviour. But since the feature is turned off by
 default, I decided that whoever is using it, is aware of cost. Instead of
 truncating the history automatically (which could lead to a nasty
 surprise), I decided to equip the user with \ansclean , a command erasing
 the history. I believe that it is better to let the user decide when
 history should be erased, instead of doing it automatically.


I think you are correct. However, if we turn on the feature by default (at
some point in the future) the discussion should probably be re-visited.

This is  my first submitted patch, so I can't really comment on the
 process. But if you could add the author's email to CC, the message would
 be much easier to spot. I replied after two days only because I missed the
 message in the flood of other pgsql-hacker messages. I think I need to scan
 the list more carefully...

My fault, I definitely should have CC'd you.

As for the patch, I made a new version of the latest one you provided in
the original thread. Let me know if anything breaks, but it compiles fine
on my box. Thanks for the feedback!

Ian


query-history-review1.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] Review: query result history in psql

2013-06-28 Thread Pavel Stehule
Hello

after patching I god segfault

Program terminated with signal 11, Segmentation fault.
#0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
98  for (p = prompt_string;
Missing separate debuginfos, use: debuginfo-install glibc-2.13-2.i686
ncurses-libs-5.7-9.20100703.fc14.i686 readline-6.1-2.fc14.i386
(gdb) bt
#0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
#1  0x0805786a in MainLoop (source=0xc45440) at mainloop.c:134
#2  0x0805a68d in main (argc=2, argv=0xbfcf2894) at startup.c:336

Regards

Pavel Stehule

2013/6/28 ian link i...@ilink.io:
 It's better to post a review as a reply to the message which contains
 the patch.

 Sorry about that, I did not have the email in my inbox and couldn't figure
 out how to use the old message ID to send a reply. Here is the thread:
 http://www.postgresql.org/message-id/flat/caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com#caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com

 The 'EscapeForCopy' was meant to mean 'Escape string in a format require
 by the COPY TEXT format', so 'copy' in the name refers to the escaping
 format, not the action performed by the function.


 I see, that makes sense now. Keep it as you see fit, it's not a big deal in
 my opinion.

  Some mathematical toolkits, like Matlab or Mathematica, automatically set
 a variable called 'ans' (short for answer) containing the result of the
 last operation. I was trying to emulate exactly this behaviour.


 I've actually been using Matlab lately, which must be why the name made
 sense to me intuitively. I don't know if this is the best name, however. It
 kind of assumes that our users use Matlab/Octave/Mathematica. Maybe 'qhist'
 or 'hist' or something?

 The history is not erased. The history is always stored in the client's
 memory.

 Ah, I did not pick up on that. Thank you for explaining it! That's actually
 a very neat way of doing it. Sorry I did not realize that at first.

 I was considering such a behaviour. But since the feature is turned off by
 default, I decided that whoever is using it, is aware of cost. Instead of
 truncating the history automatically (which could lead to a nasty surprise),
 I decided to equip the user with \ansclean , a command erasing the history.
 I believe that it is better to let the user decide when history should be
 erased, instead of doing it automatically.


 I think you are correct. However, if we turn on the feature by default (at
 some point in the future) the discussion should probably be re-visited.

 This is  my first submitted patch, so I can't really comment on the
 process. But if you could add the author's email to CC, the message would be
 much easier to spot. I replied after two days only because I missed the
 message in the flood of other pgsql-hacker messages. I think I need to scan
 the list more carefully...

 My fault, I definitely should have CC'd you.

 As for the patch, I made a new version of the latest one you provided in the
 original thread. Let me know if anything breaks, but it compiles fine on my
 box. Thanks for the feedback!

 Ian


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



-- 
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] Review: query result history in psql

2013-06-28 Thread ian link
I just applied the patch to a clean branch from the latest master. I
couldn't get a segfault from using the new feature. Could you provide a
little more info to reproduce the segfault? Thanks


On Thu, Jun 27, 2013 at 11:28 PM, Pavel Stehule pavel.steh...@gmail.comwrote:

 Hello

 after patching I god segfault

 Program terminated with signal 11, Segmentation fault.
 #0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
 98  for (p = prompt_string;
 Missing separate debuginfos, use: debuginfo-install glibc-2.13-2.i686
 ncurses-libs-5.7-9.20100703.fc14.i686 readline-6.1-2.fc14.i386
 (gdb) bt
 #0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
 #1  0x0805786a in MainLoop (source=0xc45440) at mainloop.c:134
 #2  0x0805a68d in main (argc=2, argv=0xbfcf2894) at startup.c:336

 Regards

 Pavel Stehule

 2013/6/28 ian link i...@ilink.io:
  It's better to post a review as a reply to the message which contains
  the patch.
 
  Sorry about that, I did not have the email in my inbox and couldn't
 figure
  out how to use the old message ID to send a reply. Here is the thread:
 
 http://www.postgresql.org/message-id/flat/caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com#caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com
 
  The 'EscapeForCopy' was meant to mean 'Escape string in a format require
  by the COPY TEXT format', so 'copy' in the name refers to the escaping
  format, not the action performed by the function.
 
 
  I see, that makes sense now. Keep it as you see fit, it's not a big deal
 in
  my opinion.
 
   Some mathematical toolkits, like Matlab or Mathematica, automatically
 set
  a variable called 'ans' (short for answer) containing the result of
 the
  last operation. I was trying to emulate exactly this behaviour.
 
 
  I've actually been using Matlab lately, which must be why the name made
  sense to me intuitively. I don't know if this is the best name, however.
 It
  kind of assumes that our users use Matlab/Octave/Mathematica. Maybe
 'qhist'
  or 'hist' or something?
 
  The history is not erased. The history is always stored in the client's
  memory.
 
  Ah, I did not pick up on that. Thank you for explaining it! That's
 actually
  a very neat way of doing it. Sorry I did not realize that at first.
 
  I was considering such a behaviour. But since the feature is turned off
 by
  default, I decided that whoever is using it, is aware of cost. Instead
 of
  truncating the history automatically (which could lead to a nasty
 surprise),
  I decided to equip the user with \ansclean , a command erasing the
 history.
  I believe that it is better to let the user decide when history should
 be
  erased, instead of doing it automatically.
 
 
  I think you are correct. However, if we turn on the feature by default
 (at
  some point in the future) the discussion should probably be re-visited.
 
  This is  my first submitted patch, so I can't really comment on the
  process. But if you could add the author's email to CC, the message
 would be
  much easier to spot. I replied after two days only because I missed the
  message in the flood of other pgsql-hacker messages. I think I need to
 scan
  the list more carefully...
 
  My fault, I definitely should have CC'd you.
 
  As for the patch, I made a new version of the latest one you provided in
 the
  original thread. Let me know if anything breaks, but it compiles fine on
 my
  box. Thanks for the feedback!
 
  Ian
 
 
  --
  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] Review: query result history in psql

2013-06-28 Thread Pavel Stehule
Hello

It's look like my bug - wrong compilation

I am sorry

Pavel

2013/6/28 ian link i...@ilink.io:
 I just applied the patch to a clean branch from the latest master. I
 couldn't get a segfault from using the new feature. Could you provide a
 little more info to reproduce the segfault? Thanks


 On Thu, Jun 27, 2013 at 11:28 PM, Pavel Stehule pavel.steh...@gmail.com
 wrote:

 Hello

 after patching I god segfault

 Program terminated with signal 11, Segmentation fault.
 #0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
 98  for (p = prompt_string;
 Missing separate debuginfos, use: debuginfo-install glibc-2.13-2.i686
 ncurses-libs-5.7-9.20100703.fc14.i686 readline-6.1-2.fc14.i386
 (gdb) bt
 #0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
 #1  0x0805786a in MainLoop (source=0xc45440) at mainloop.c:134
 #2  0x0805a68d in main (argc=2, argv=0xbfcf2894) at startup.c:336

 Regards

 Pavel Stehule

 2013/6/28 ian link i...@ilink.io:
  It's better to post a review as a reply to the message which contains
  the patch.
 
  Sorry about that, I did not have the email in my inbox and couldn't
  figure
  out how to use the old message ID to send a reply. Here is the thread:
 
  http://www.postgresql.org/message-id/flat/caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com#caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com
 
  The 'EscapeForCopy' was meant to mean 'Escape string in a format
  require
  by the COPY TEXT format', so 'copy' in the name refers to the escaping
  format, not the action performed by the function.
 
 
  I see, that makes sense now. Keep it as you see fit, it's not a big deal
  in
  my opinion.
 
   Some mathematical toolkits, like Matlab or Mathematica, automatically
  set
  a variable called 'ans' (short for answer) containing the result of
  the
  last operation. I was trying to emulate exactly this behaviour.
 
 
  I've actually been using Matlab lately, which must be why the name made
  sense to me intuitively. I don't know if this is the best name, however.
  It
  kind of assumes that our users use Matlab/Octave/Mathematica. Maybe
  'qhist'
  or 'hist' or something?
 
  The history is not erased. The history is always stored in the client's
  memory.
 
  Ah, I did not pick up on that. Thank you for explaining it! That's
  actually
  a very neat way of doing it. Sorry I did not realize that at first.
 
  I was considering such a behaviour. But since the feature is turned off
  by
  default, I decided that whoever is using it, is aware of cost. Instead
  of
  truncating the history automatically (which could lead to a nasty
  surprise),
  I decided to equip the user with \ansclean , a command erasing the
  history.
  I believe that it is better to let the user decide when history should
  be
  erased, instead of doing it automatically.
 
 
  I think you are correct. However, if we turn on the feature by default
  (at
  some point in the future) the discussion should probably be re-visited.
 
  This is  my first submitted patch, so I can't really comment on the
  process. But if you could add the author's email to CC, the message
  would be
  much easier to spot. I replied after two days only because I missed the
  message in the flood of other pgsql-hacker messages. I think I need to
  scan
  the list more carefully...
 
  My fault, I definitely should have CC'd you.
 
  As for the patch, I made a new version of the latest one you provided in
  the
  original thread. Let me know if anything breaks, but it compiles fine on
  my
  box. Thanks for the feedback!
 
  Ian
 
 
  --
  Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
  To make changes to your subscription:
  http://www.postgresql.org/mailpref/pgsql-hackers
 




-- 
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] Review: query result history in psql

2013-06-28 Thread ian link
No worries! :)


On Fri, Jun 28, 2013 at 12:20 AM, Pavel Stehule pavel.steh...@gmail.comwrote:

 Hello

 It's look like my bug - wrong compilation

 I am sorry

 Pavel

 2013/6/28 ian link i...@ilink.io:
  I just applied the patch to a clean branch from the latest master. I
  couldn't get a segfault from using the new feature. Could you provide a
  little more info to reproduce the segfault? Thanks
 
 
  On Thu, Jun 27, 2013 at 11:28 PM, Pavel Stehule pavel.steh...@gmail.com
 
  wrote:
 
  Hello
 
  after patching I god segfault
 
  Program terminated with signal 11, Segmentation fault.
  #0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
  98  for (p = prompt_string;
  Missing separate debuginfos, use: debuginfo-install glibc-2.13-2.i686
  ncurses-libs-5.7-9.20100703.fc14.i686 readline-6.1-2.fc14.i386
  (gdb) bt
  #0  0x0805aab4 in get_prompt (status=PROMPT_READY) at prompt.c:98
  #1  0x0805786a in MainLoop (source=0xc45440) at mainloop.c:134
  #2  0x0805a68d in main (argc=2, argv=0xbfcf2894) at startup.c:336
 
  Regards
 
  Pavel Stehule
 
  2013/6/28 ian link i...@ilink.io:
   It's better to post a review as a reply to the message which contains
   the patch.
  
   Sorry about that, I did not have the email in my inbox and couldn't
   figure
   out how to use the old message ID to send a reply. Here is the thread:
  
  
 http://www.postgresql.org/message-id/flat/caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com#caecsyxjri++t3pevdyzawh2ygx7kg9zrhx8kawtp1fxv3h0...@mail.gmail.com
  
   The 'EscapeForCopy' was meant to mean 'Escape string in a format
   require
   by the COPY TEXT format', so 'copy' in the name refers to the
 escaping
   format, not the action performed by the function.
  
  
   I see, that makes sense now. Keep it as you see fit, it's not a big
 deal
   in
   my opinion.
  
Some mathematical toolkits, like Matlab or Mathematica,
 automatically
   set
   a variable called 'ans' (short for answer) containing the result of
   the
   last operation. I was trying to emulate exactly this behaviour.
  
  
   I've actually been using Matlab lately, which must be why the name
 made
   sense to me intuitively. I don't know if this is the best name,
 however.
   It
   kind of assumes that our users use Matlab/Octave/Mathematica. Maybe
   'qhist'
   or 'hist' or something?
  
   The history is not erased. The history is always stored in the
 client's
   memory.
  
   Ah, I did not pick up on that. Thank you for explaining it! That's
   actually
   a very neat way of doing it. Sorry I did not realize that at first.
  
   I was considering such a behaviour. But since the feature is turned
 off
   by
   default, I decided that whoever is using it, is aware of cost.
 Instead
   of
   truncating the history automatically (which could lead to a nasty
   surprise),
   I decided to equip the user with \ansclean , a command erasing the
   history.
   I believe that it is better to let the user decide when history
 should
   be
   erased, instead of doing it automatically.
  
  
   I think you are correct. However, if we turn on the feature by default
   (at
   some point in the future) the discussion should probably be
 re-visited.
  
   This is  my first submitted patch, so I can't really comment on the
   process. But if you could add the author's email to CC, the message
   would be
   much easier to spot. I replied after two days only because I missed
 the
   message in the flood of other pgsql-hacker messages. I think I need
 to
   scan
   the list more carefully...
  
   My fault, I definitely should have CC'd you.
  
   As for the patch, I made a new version of the latest one you provided
 in
   the
   original thread. Let me know if anything breaks, but it compiles fine
 on
   my
   box. Thanks for the feedback!
  
   Ian
  
  
   --
   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] Review: query result history in psql

2013-06-28 Thread Maciej Gajewski
Thanks for checking the patch!

So what's left to fix?
* Moving the escaping-related functions to separate module,
* applying your corrections.

Did I missed anything?

I'll submit corrected patch after the weekend.

M


Re: [HACKERS] Review: query result history in psql

2013-06-28 Thread Pavel Stehule
Hello

I am not sure, this interface is really user friendly

there is not possible searching in history, and not every query push
to history some interesting content.

It require:

* simply decision if content should be stored in history or not,
* simply remove last entry (table) of history
* queries should be joined to content, only name is not enough

Regards

Pavel

2013/6/28 Maciej Gajewski maciej.gajews...@gmail.com:
 Thanks for checking the patch!

 So what's left to fix?
 * Moving the escaping-related functions to separate module,
 * applying your corrections.

 Did I missed anything?

 I'll submit corrected patch after the weekend.

 M



-- 
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] Review: query result history in psql

2013-06-27 Thread Maciej Gajewski
Thank you for the review!


There were a few english/grammatical mistakes that I went ahead and fixed.


Thank  you for that. If you could send me a patch-to-a-patch so I can
correct all the mistakes in the next release?


 Additionally, I think some of the string manipulation might be placed
 outside of the main ans.c file. I don't know if there's a better place for
 'EscapeForCopy' and 'GetEscapedLen'. Not really a big deal, just an
 organizational idea. I also changed 'EscapeForCopy' to 'EscapeAndCopy'. I
 think that better describes the functionality. 'EscapeForCopy' kind of
 implies that another function is needed to copy the string.



The 'EscapeForCopy' was meant to mean 'Escape string in a format require by
the COPY TEXT format', so 'copy' in the name refers to the escaping format,
not the action performed by the function.

They could be, indeed, placed in separate module. I'll do it.



 What does 'ans' stand for? I am not sure how it relates to the concept of
 a query history. It didn't stop my understanding of the code, but I don't
 know if a user will immediately know the meaning.


Some mathematical toolkits, like Matlab or Mathematica, automatically set a
variable called 'ans' (short for answer) containing the result of the
last operation. I was trying to emulate exactly this behaviour.



 Probably the biggest problem is that the query history list is missing a
 maximum size variable. I think this could be valuable for preventing users
 from shooting themselves in the foot. If the user is running large queries,
 they might accidentally store too much data. This probably somewhat of an
 edge-case but I believe it is worth considering. We could provide a
 sensible default limit (10 queries?) and also allow the user to change it.


I was considering such a behaviour. But since the feature is turned off by
default, I decided that whoever is using it, is aware of cost. Instead of
truncating the history automatically (which could lead to a nasty
surprise), I decided to equip the user with \ansclean , a command erasing
the history. I believe that it is better to let the user decide when
history should be erased, instead of doing it automatically.


Finally, is it worth resetting the query history every time a user
 reconnects to the database? I can see how this might interrupt a user's
 workflow. If the user suddenly disconnects (network connection interrupted,
 etc) then they would lose their history. I think this is definitely up for
 debate. It would add more management overhead (psql options etc) and might
 just be unnecessary. However, with a sane limit to the size of the query
 history, I don't know if there would be too many drawbacks from a storage
 perspective.


The history is not erased. The history is always stored in the client's
memory. When a history item is used for the first time, a TEMPORARY table
is created in the database that stores the data server-side. When user
disconnects from the database, the session ends and all these tables are
dropped.
Tables names have to be removed from the history, so next time the item is
used, the table will be created and populated again.

I use the feature while switching often between databases, and it works
seamlessly. Actually, it's quite useful to move bits of data across
databases:
Connect to database A, run a query, connect to database B, run another
query joining local data with the results of the previous query.


 Those issues aside - I think it's a great feature! I can add the
 grammatical fixes I made whenever the final patch is ready. Or earlier,
 whatever works for you. Also, this is my first time reviewing a patch, so
 please let me know if I can improve on anything. Thanks!


This is  my first submitted patch, so I can't really comment on the
process. But if you could add the author's email to CC, the message would
be much easier to spot. I replied after two days only because I missed the
message in the flood of other pgsql-hacker messages. I think I need to scan
the list more carefully...

Maciej


Re: [HACKERS] Review: query result history in psql

2013-06-27 Thread Alvaro Herrera
Maciej Gajewski escribió:

  Those issues aside - I think it's a great feature! I can add the
  grammatical fixes I made whenever the final patch is ready. Or earlier,
  whatever works for you. Also, this is my first time reviewing a patch, so
  please let me know if I can improve on anything. Thanks!
 
 This is  my first submitted patch, so I can't really comment on the
 process. But if you could add the author's email to CC, the message would
 be much easier to spot. I replied after two days only because I missed the
 message in the flood of other pgsql-hacker messages. I think I need to scan
 the list more carefully...

It's better to post a review as a reply to the message which contains
the patch.

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