Review: query result history in psql

Started by ian linkover 12 years ago25 messages
#1ian link
ian@ilink.io

Hi Maciej,

I've been reviewing your patch for the ongoing commitfest. First let me say
that I think it's a great idea and provides some very useful functionality.

However, there are a few minor problems. There were a few
english/grammatical mistakes that I went ahead and fixed. 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.

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.

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.

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.

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!

Ian Link

#2Maciej Gajewski
maciej.gajewski0@gmail.com
In reply to: ian link (#1)
Re: Review: query result history in psql

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

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Maciej Gajewski (#2)
Re: Review: query result history in psql

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 Herrera http://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

#4ian link
ian@ilink.io
In reply to: Alvaro Herrera (#3)
1 attachment(s)
Re: Review: query result history in psql

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:
/messages/by-id/CAEcSYXJRi++T3pevDyzAWH2yGx7kG9ZrhX8KAWtP1fXV3H02vw@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

Attachments:

query-history-review1.patchapplication/octet-stream; name=query-history-review1.patchDownload
*** a/src/bin/psql/Makefile
--- b/src/bin/psql/Makefile
***************
*** 20,26 **** REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref
  
  override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) -I$(top_srcdir)/src/bin/pg_dump $(CPPFLAGS)
  
! OBJS=	command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
  	startup.o prompt.o variables.o large_obj.o print.o describe.o \
  	tab-complete.o mbprint.o dumputils.o keywords.o kwlookup.o \
  	sql_help.o \
--- 20,26 ----
  
  override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) -I$(top_srcdir)/src/bin/pg_dump $(CPPFLAGS)
  
! OBJS=	ans.o command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
  	startup.o prompt.o variables.o large_obj.o print.o describe.o \
  	tab-complete.o mbprint.o dumputils.o keywords.o kwlookup.o \
  	sql_help.o \
*** /dev/null
--- b/src/bin/psql/ans.c
***************
*** 0 ****
--- 1,462 ----
+ /*
+  * psql - the PostgreSQL interactive terminal
+  *
+  * Copyright (c) 2013, PostgreSQL Global Development Group
+  *
+  * src/bin/psql/ans.c
+  */
+ 
+ #include "ans.h"
+ 
+ #include "postgres_fe.h"
+ 
+ static int global_ans_num = 0;
+ 
+ static void CreateTable(PGconn *db, struct _ans* item);
+ static char* GetTypeName(PGconn *db, Oid oid);
+ static char* BuildData(PGresult* result);
+ 
+ /*
+  * Creates empty entry, serving as list head
+  */
+ AnsHistory CreateAnsHistory(void)
+ {
+ 	struct _ans* head;
+ 	
+ 	head = pg_malloc(sizeof(struct _ans));
+ 	
+ 	head->numColumns = 0;
+ 	head->columnTypes = NULL;
+ 	head->columnNames = NULL;
+ 	head->data = NULL;
+ 	head->next = NULL;
+ 	head->tableName = NULL;
+ 	head->name = NULL;
+ 	
+ 	return head;
+ }
+ 
+ void
+ AddToHistory(AnsHistory history, PGresult* result)
+ {
+ 	struct _ans* item;
+ 	int numRows, numColumns;
+ 	int i;
+ 	char* data;
+ 	
+ 	if (!history || !result)
+ 		return;
+ 	
+ 	numColumns  = PQnfields(result);
+ 	numRows = PQntuples(result);
+ 	
+ 	if (numColumns <= 0 || numRows <= 0)
+ 		return;
+ 	
+ 	data = BuildData(result);
+ 	if (!data)
+ 		return;
+ 	
+ 	item = pg_malloc(sizeof(struct _ans));
+ 
+ 	/* read meta-data: column names and types */
+ 	item->numColumns = numColumns;
+ 	item->columnNames = pg_malloc(sizeof(char*) * item->numColumns);
+ 	item->columnTypes = pg_malloc(sizeof(Oid) * item->numColumns);
+ 	
+ 	for (i = 0; i < item->numColumns; i++)
+ 	{
+ 		char* name;
+ 		
+ 		item->columnTypes[i] = PQftype(result, i);
+ 		
+ 		name = PQfname(result, i);
+ 		item->columnNames[i] = pg_malloc(strlen(name) + 1);
+ 		strcpy(item->columnNames[i], name);
+ 	}
+ 	
+ 	item->data = data;
+ 	
+ 	/* Table creation deferred to when the ASN is actually used */
+ 	item->tableName = NULL;
+ 
+ 	item->name = pg_malloc(10);
+ 	sprintf(item->name, "ans%d", global_ans_num++);
+ 	
+ 	printf(_("Query result stored as :%s\n"), item->name);
+ 	
+ 	item->next = history->next;
+ 	history->next = item;
+ }
+ 
+ const char*
+ GetOrCreateTable(AnsHistory history, PGconn *db, const char* name)
+ {
+ 	struct _ans* item;
+ 	
+ 	if (!history || !name)
+ 	{
+ 		return NULL;
+ 	}
+ 	
+ 	item = history->next;
+ 	
+ 	while (item)
+ 	{
+ 		if (strcmp(item->name, name) == 0)
+ 		{
+ 			if (item->tableName)
+ 				return item->tableName;
+ 			
+ 			CreateTable(db, item);
+ 			if (item->tableName)
+ 				return item->tableName;
+ 			else
+ 				return NULL;
+ 		}
+ 		item = item->next;
+ 	}
+ 	
+ 	return NULL;
+ }
+ 
+ static
+ void 
+ CreateTable(PGconn *db, struct _ans* item)
+ {
+ 	char tableNameBuf[32];
+ 	char copyQuery[64];
+ 	const int qbufsize=2048;
+ 	char queryBuf[qbufsize];
+ 	char* queryPtr;
+ 	int len;
+ 	int i;
+ 	PGresult* qres;
+ 	
+ 	if (!item)
+ 		return;
+ 	
+ 	len = snprintf(tableNameBuf, 32, "_table_%s", item->name);
+ 	if (len >= 32)
+ 	{
+ 		return;
+ 	}
+ 	len = snprintf(copyQuery, 64, "COPY %s FROM STDIN", tableNameBuf);
+ 	if (len >= 64)
+ 	{
+ 		return;
+ 	}
+ 	
+ 	/* Build CREATE TABLE query */
+ 	queryPtr = queryBuf;
+ 	queryPtr += snprintf(queryPtr, qbufsize-(int)(queryPtr-queryBuf), "CREATE TEMP TABLE %s (\n", tableNameBuf);
+ 	
+ 	for (i = 0; i < item->numColumns; ++i)
+ 	{
+ 		char* typeName;
+ 		const char* format;
+ 		
+ 		typeName = GetTypeName(db, item->columnTypes[i]);
+ 		if (!typeName)
+ 			return;
+ 		
+ 		if (i == item->numColumns-1)
+ 			format = "%s %s\n";
+ 		else
+ 			format = "%s %s,\n";
+ 			
+ 		queryPtr += snprintf(queryPtr, qbufsize-(int)(queryPtr-queryBuf), format, item->columnNames[i], typeName);
+ 		free(typeName);
+ 		if (queryPtr >= queryBuf+qbufsize)
+ 		{
+ 			return; 
+ 		}
+ 	}
+ 	queryPtr += snprintf(queryPtr, qbufsize-(int)(queryPtr-queryBuf), ");");
+ 	if (queryPtr >= queryBuf+qbufsize)
+ 	{
+ 		return; 
+ 	}
+ 	
+ 	/* create and populate table */
+ 	PQclear(PQexec(db, "BEGIN"));
+ 	qres = PQexec(db, queryBuf);
+ 	if (PQresultStatus(qres) != PGRES_COMMAND_OK)
+ 	{
+ 		PQclear(qres);
+ 		PQclear(PQexec(db, "ROLLBACK"));
+ 		return;
+ 	}
+ 	PQclear(qres);
+ 
+ 	qres = PQexec(db, copyQuery);
+ 	if (PQresultStatus(qres) != PGRES_COPY_IN)
+ 	{
+ 		PQclear(qres);
+ 		PQclear(PQexec(db, "ROLLBACK"));
+ 		return;
+ 	}
+ 	PQclear(qres);
+ 	
+ 	PQputCopyData(db, item->data, strlen(item->data));
+ 	len = PQputCopyEnd(db, NULL);
+ 	if (len != 1)
+ 	{
+ 		PQclear(PQexec(db, "ROLLBACK"));
+ 		return;
+ 	}
+ 	qres = PQgetResult(db);
+ 	if (PQresultStatus(qres) != PGRES_COMMAND_OK)
+ 	{
+ 		PQclear(qres);
+ 		PQclear(PQexec(db, "ROLLBACK"));
+ 		return;
+ 	}
+ 	PQclear(qres);
+ 	PQclear(PQexec(db, "COMMIT"));
+ 	
+ 	item->tableName = pg_strdup(tableNameBuf);
+ }
+ 
+ /* free the typename after use */
+ static
+ char* 
+ GetTypeName(PGconn *db, Oid oid)
+ {
+ 	const int bufsize = 1024;
+ 	char queryBuf[bufsize];
+ 	int sres;
+ 	PGresult* qres;
+ 	char* typeName;
+ 	
+ 	sres = snprintf(queryBuf, bufsize, "SELECT typname FROM pg_type WHERE oid=%d;", oid);
+ 	if ( sres < 0 || sres > bufsize)
+ 		return NULL;
+ 	
+ 	qres = PQexec(db, queryBuf);
+ 	if (!qres)
+ 		return NULL;
+ 	
+ 	if (PQresultStatus(qres) != PGRES_TUPLES_OK)
+ 	{
+ 		PQclear(qres);
+ 		return NULL;
+ 	}
+ 	
+ 	if (PQntuples(qres) != 1 && PQnfields(qres) != 1)
+ 	{
+ 		PQclear(qres);
+ 		return NULL;
+ 	}
+ 
+ 	typeName = pg_strdup(PQgetvalue(qres, 0, 0));
+ 	PQclear(qres);
+ 	return typeName;
+ }
+ 
+ /* Returns length of string after escaping */
+ static
+ int GetEscapedLen(const char* c)
+ {
+ 	int len = 0;
+ 	for(; *c; c++)
+ 	{
+ 		switch(*c)
+ 		{
+ 			case '\\':
+ 			case '\b':
+ 			case '\f':
+ 			case '\r':
+ 			case '\n':
+ 			case '\t':
+ 			case '\v':
+ 				len++;
+ 			default:
+ 				len++;
+ 		}
+ 	}
+ 	
+ 	return len;
+ }
+ 
+ /* Copies c to dst, escaping. Returns pointer past the last character copied.
+  * Assumes there is enough room in the dst buffer
+  */
+ static
+ char*
+ EscapeForCopy(char* dst, const char* c)
+ {
+ 	for(; *c; c++)
+ 	{
+ 		switch(*c)
+ 		{
+ 			case '\\':
+ 				*(dst++) = '\\';
+ 				*(dst++) = '\\';
+ 				break;
+ 				
+ 			case '\b':
+ 				*(dst++) = '\\';
+ 				*(dst++) = 'b';
+ 				break;
+ 				
+ 			case '\f':
+ 				*(dst++) = '\\';
+ 				*(dst++) = 'f';
+ 				break;
+ 				
+ 			case '\r':
+ 				*(dst++) = '\\';
+ 				*(dst++) = 'r';
+ 				break;
+ 				
+ 			case '\n':
+ 				*(dst++) = '\\';
+ 				*(dst++) = 'n';
+ 				break;
+ 				
+ 			case '\t':
+ 				*(dst++) = '\\';
+ 				*(dst++) = 't';
+ 				break;
+ 				
+ 			case '\v':
+ 				*(dst++) = '\\';
+ 				*(dst++) = 'v';
+ 				break;
+ 				
+ 			default:
+ 				*(dst++) = *c;
+ 		}
+ 	}
+ 	return dst;
+ }
+ 
+ /* Convert query data into data buffer in COPY TEXT format.
+  * returns NULL on error
+  * caller owns the data
+  */
+ static 
+ char* 
+ BuildData(PGresult* result)
+ {
+ 	int cols, rows;
+ 	int bufferSize;
+ 	int r,c;
+ 	char* buffer;
+ 	char* dataPtr;
+ 	
+ 	if (!result)
+ 		return NULL;
+ 	
+ 	cols = PQnfields(result);
+ 	rows = PQntuples(result);
+ 	
+ 	/* first pass - measure the size */
+ 	bufferSize = 0;
+ 	for(r = 0; r < rows; r++)
+ 	{
+ 		for(c = 0; c < cols; c++)
+ 		{
+ 			if (PQgetisnull(result, r, c))
+ 			{
+ 				bufferSize += 2; /* 2 = size of null literal \N */
+ 			}
+ 			else
+ 			{
+ 				bufferSize += GetEscapedLen(PQgetvalue(result, r, c));
+ 			}
+ 			bufferSize +=1; /* column or row delimiter*/
+ 		}
+ 	}
+ 	bufferSize += 4; /* end-of-data marker \. + NULL terminiator */
+ 	
+ 	/* second pass = build the buffer */
+ 	buffer = pg_malloc(bufferSize);
+ 	dataPtr = buffer;
+ 	
+ 	for(r = 0; r < rows; r++)
+ 	{
+ 		for(c = 0; c < cols; c++)
+ 		{
+ 			if (PQgetisnull(result, r, c))
+ 			{
+ 				strcpy(dataPtr, "\\N");
+ 				dataPtr += 2;
+ 			}
+ 			else
+ 			{
+ 				char* value = PQgetvalue(result, r, c);
+ 				dataPtr = EscapeForCopy(dataPtr, value);
+ 			}
+ 			if (c == cols-1)
+ 			{
+ 				strcpy(dataPtr, "\n");
+ 			}
+ 			else
+ 			{
+ 				strcpy(dataPtr, "\t");
+ 			}
+ 			dataPtr += 1;
+ 		}
+ 	}
+ 	strcpy(dataPtr, "\\.\n");
+ 	
+ 	return buffer;
+ }
+ 
+ void AnsClearTableNames(AnsHistory history)
+ {
+ 	struct _ans* item = history;
+ 	
+ 	while(item->next)
+ 	{
+ 		item = item->next;
+ 		
+ 		if (item->tableName)
+ 		{
+ 			free(item->tableName);
+ 			item->tableName = NULL;
+ 		}
+ 	}
+ 	
+ }
+ 
+ void
+ DestroyAnsHistory(PGconn *db, AnsHistory item)
+ {
+ 	if (!item)
+ 		return;
+ 	
+ 	if (item->next)
+ 		DestroyAnsHistory(db, item->next);
+ 	
+ 	if (item->data)
+ 		free(item->data);
+ 	
+ 	if(item->name)
+ 		free(item->name);
+ 	
+ 	if (item->columnTypes)
+ 		free(item->columnTypes);
+ 	
+ 	if (item->numColumns && item->columnNames)
+ 	{
+ 		int i;
+ 		for(i = 0; i < item->numColumns; i++)
+ 			free(item->columnNames[i]);
+ 		
+ 		free(item->columnNames);
+ 	}
+ 	
+ 	if (item->tableName)
+ 	{
+ 		const int bufsize = 32;
+ 		char deleteQuery[bufsize];
+ 
+ 		snprintf(deleteQuery, bufsize, "DROP TABLE %s", item->tableName);
+ 		PQclear(PQexec(db, deleteQuery));
+ 	}
+ 	
+ 	free(item);
+ }
*** /dev/null
--- b/src/bin/psql/ans.h
***************
*** 0 ****
--- 1,48 ----
+ /*
+  * psql - the PostgreSQL interactive terminal
+  *
+  * Copyright (c) 2013, PostgreSQL Global Development Group
+  *
+  * src/bin/psql/ans.h
+  */
+ #ifndef ANS_H
+ #define ANS_H
+ 
+ #include "libpq-fe.h"
+ 
+ 
+ /* This forms a list of the last N query results. The format allows us to insert into a temporary table*/
+ struct _ans 
+ {
+ 	int		numColumns;
+ 	Oid		*columnTypes;
+ 	char	**columnNames;
+ 	char	*data;
+ 	
+ 	char	*name;
+ 	char	*tableName; // corresponding table in the DB. NULL if not created yet
+ 	
+ 	struct _ans	*next;
+ };
+ 
+ typedef struct _ans* AnsHistory;
+ 
+ AnsHistory	CreateAnsHistory(void);
+ void		AddToHistory(AnsHistory history, PGresult* result);
+ void		DestroyAnsHistory(PGconn *db, AnsHistory item);
+ 
+ 
+ /*
+  * Checks if the provided name exists in the history.
+  * If none found, returns NULL.
+  * If found, but table is not created yet, it creates the table first.
+  */
+ const char* GetOrCreateTable(AnsHistory history, PGconn *db, const char* name);
+ 
+ /* Should be called when a new connection is opened.
+  * All the ANS tables are temporary. This means they must be cleared if the client connects
+  * to a new database.
+  */
+ void AnsClearTableNames(AnsHistory history);
+ 
+ #endif
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***************
*** 40,45 ****
--- 40,46 ----
  #include "pqexpbuffer.h"
  #include "dumputils.h"
  
+ #include "ans.h"
  #include "common.h"
  #include "copy.h"
  #include "describe.h"
***************
*** 51,56 ****
--- 52,58 ----
  #include "psqlscan.h"
  #include "settings.h"
  #include "variables.h"
+ #include "ans.h"
  
  
  /* functions for use in this file */
***************
*** 205,210 **** exec_command(const char *cmd,
--- 207,247 ----
  			success = do_pset("format", "unaligned", &pset.popt, pset.quiet);
  	}
  
+ 	/* \ans - toggle query result history */
+ 	else if (strcmp(cmd, "ans") == 0)
+ 	{
+ 		char	 *opt = psql_scan_slash_option(scan_state,
+ 												 OT_NORMAL, NULL, false);
+ 
+ 		if (opt)
+ 			pset.ans_enabled = ParseVariableBool(opt);
+ 		else
+ 			// convenience: toggle the setting if the user did not provide an option
+ 			pset.ans_enabled = !pset.ans_enabled;
+ 		
+ 		if (!pset.quiet)
+ 		{
+ 			if (pset.ans_enabled)
+ 				puts(_("Query result history is on."));
+ 			else
+ 			{
+ 				puts(_("Query result history is off."));
+ 			}
+ 		}
+ 		free(opt);
+ 	}
+ 	/* \ansclean - clear query result history */
+ 	else if (strcmp(cmd, "ansclean") == 0)
+ 	{
+ 		DestroyAnsHistory(pset.db, pset.ans);
+ 		pset.ans = CreateAnsHistory();
+ 		
+ 		if (!pset.quiet)
+ 		{
+ 			puts(_("Query result history cleaned."));
+ 		}
+ 	}
+ 
  	/* \C -- override table title (formerly change HTML caption) */
  	else if (strcmp(cmd, "C") == 0)
  	{
***************
*** 1697,1702 **** do_connect(char *dbname, char *user, char *host, char *port)
--- 1734,1740 ----
  	PQsetNoticeProcessor(n_conn, NoticeProcessor, NULL);
  	pset.db = n_conn;
  	SyncVariables();
+ 	AnsClearTableNames(pset.ans);
  	connection_warnings(false); /* Must be after SyncVariables */
  
  	/* Tell the user about the new connection */
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
***************
*** 23,28 ****
--- 23,29 ----
  #include "command.h"
  #include "copy.h"
  #include "mbprint.h"
+ #include "ans.h"
  
  
  
***************
*** 950,956 **** SendQuery(const char *query)
--- 951,961 ----
  
  		/* but printing results isn't: */
  		if (OK && results)
+ 		{
  			OK = PrintQueryResults(results);
+ 			if (pset.ans_enabled)
+ 				AddToHistory(pset.ans, results);
+ 		}
  	}
  	else
  	{
***************
*** 1227,1233 **** ExecQueryUsingCursor(const char *query, double *elapsed_msec)
  		}
  
  		printQuery(results, &my_popt, pset.queryFout, pset.logfile);
! 
  		PQclear(results);
  
  		/* after the first result set, disallow header decoration */
--- 1232,1238 ----
  		}
  
  		printQuery(results, &my_popt, pset.queryFout, pset.logfile);
! 		
  		PQclear(results);
  
  		/* after the first result set, disallow header decoration */
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
***************
*** 289,294 **** slashUsage(unsigned short int pager)
--- 289,299 ----
  					  "  \\lo_import FILE [COMMENT]\n"
  					  "  \\lo_list\n"
  					  "  \\lo_unlink LOBOID      large object operations\n"));
+ 	fprintf(output, "\n");
+ 
+ 	fprintf(output, _("Query result history\n"));
+ 	fprintf(output, _("  \\ans [on|off]          toggle query result history (currently %s)\n"), ON(pset.ans_enabled));
+ 	fprintf(output, _("  \\ansclean              clean query result history\n"));
  
  	ClosePager(output);
  }
*** a/src/bin/psql/psqlscan.l
--- b/src/bin/psql/psqlscan.l
***************
*** 737,747 **** other			.
  					}
  					else
  					{
! 						/*
! 						 * if the variable doesn't exist we'll copy the
! 						 * string as is
  						 */
! 						ECHO;
  					}
  
  					free(varname);
--- 737,760 ----
  					}
  					else
  					{
! 						/* 
! 						 * This might be the ANS variable
  						 */
! 						const char* tableName;
! 						
! 						tableName = GetOrCreateTable(pset.ans, pset.db, varname);
! 						if (tableName)
! 						{
! 							push_new_buffer(tableName, varname);
! 						}
! 						else
! 						{
! 							/*
! 							* if the variable does not exist, copy the
! 							* string without modification
! 							*/
! 							ECHO;
! 						}
  					}
  
  					free(varname);
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
***************
*** 11,16 ****
--- 11,17 ----
  
  #include "variables.h"
  #include "print.h"
+ #include "ans.h"
  
  #define DEFAULT_FIELD_SEP "|"
  #define DEFAULT_RECORD_SEP "\n"
***************
*** 93,98 **** typedef struct _psqlSettings
--- 94,102 ----
  	FILE	   *logfile;		/* session log file handle */
  
  	VariableSpace vars;			/* "shell variable" repository */
+ 	
+ 	AnsHistory	ans;			/* query result (answer) history.*/
+ 	bool		ans_enabled;	/* local query result storage enabled */
  
  	/*
  	 * The remaining fields are set by assign hooks associated with entries in
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
***************
*** 147,152 **** main(int argc, char *argv[])
--- 147,155 ----
  	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
  	SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
  	SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
+ 	
+ 	pset.ans = CreateAnsHistory();
+ 	pset.ans_enabled = 0;
  
  	parse_psql_options(argc, argv, &options);
  
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 891,897 **** psql_completion(char *text, int start, int end)
  	};
  
  	static const char *const backslash_commands[] = {
! 		"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
  		"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
  		"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
  		"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
--- 891,897 ----
  	};
  
  	static const char *const backslash_commands[] = {
! 		"\\a", "\\ans", "\\ansclean", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
  		"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
  		"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
  		"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: ian link (#4)
Re: Review: query result history in psql

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 <ian@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:
/messages/by-id/CAEcSYXJRi++T3pevDyzAWH2yGx7kG9ZrhX8KAWtP1fXV3H02vw@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

#6ian link
ian@ilink.io
In reply to: Pavel Stehule (#5)
Re: Review: query result history in psql

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.stehule@gmail.com>wrote:

Show quoted text

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 <ian@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:

/messages/by-id/CAEcSYXJRi++T3pevDyzAWH2yGx7kG9ZrhX8KAWtP1fXV3H02vw@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

#7Pavel Stehule
pavel.stehule@gmail.com
In reply to: ian link (#6)
Re: Review: query result history in psql

Hello

It's look like my bug - wrong compilation

I am sorry

Pavel

2013/6/28 ian link <ian@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.stehule@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 <ian@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:

/messages/by-id/CAEcSYXJRi++T3pevDyzAWH2yGx7kG9ZrhX8KAWtP1fXV3H02vw@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

#8ian link
ian@ilink.io
In reply to: Pavel Stehule (#7)
Re: Review: query result history in psql

No worries! :)

On Fri, Jun 28, 2013 at 12:20 AM, Pavel Stehule <pavel.stehule@gmail.com>wrote:

Show quoted text

Hello

It's look like my bug - wrong compilation

I am sorry

Pavel

2013/6/28 ian link <ian@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.stehule@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 <ian@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:

/messages/by-id/CAEcSYXJRi++T3pevDyzAWH2yGx7kG9ZrhX8KAWtP1fXV3H02vw@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

#9Maciej Gajewski
maciej.gajewski0@gmail.com
In reply to: ian link (#8)
Re: Review: query result history in psql

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

#10Pavel Stehule
pavel.stehule@gmail.com
In reply to: Maciej Gajewski (#9)
Re: Review: query result history in psql

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.gajewski0@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

#11ian link
ian@ilink.io
In reply to: Pavel Stehule (#10)
Re: Review: query result history in psql

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.stehule@gmail.com>wrote:

Show quoted text

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.gajewski0@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

#12Pavel Stehule
pavel.stehule@gmail.com
In reply to: ian link (#11)
Re: Review: query result history in psql

Hello

2013/7/1 ian link <ian@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.stehule@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.gajewski0@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

#13ian link
ian@ilink.io
In reply to: Pavel Stehule (#12)
Re: Review: query result history in psql

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.stehule@gmail.com>wrote:

Show quoted text

Hello

2013/7/1 ian link <ian@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.stehule@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.gajewski0@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

#14Maciej Gajewski
maciej.gajewski0@gmail.com
In reply to: ian link (#13)
Re: Review: query result history in psql

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

#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Maciej Gajewski (#14)
Re: Review: query result history in psql

2013/7/1 Maciej Gajewski <maciej.gajewski0@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

#16Maciej Gajewski
maciej.gajewski0@gmail.com
In reply to: Pavel Stehule (#15)
Re: Review: query result history in psql

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.

#17Pavel Stehule
pavel.stehule@gmail.com
In reply to: Maciej Gajewski (#16)
Re: Review: query result history in psql

2013/7/1 Maciej Gajewski <maciej.gajewski0@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

#18Ian Link
ian@ilink.io
In reply to: Pavel Stehule (#17)
2 attachment(s)
Re: Review: query result history in psql

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.

Show quoted text

Pavel Stehule <mailto:pavel.stehule@gmail.com>
Monday, July 01, 2013 4:05 AM

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

Maciej Gajewski <mailto:maciej.gajewski0@gmail.com>
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 <mailto:pavel.stehule@gmail.com>
Monday, July 01, 2013 1:31 AM
2013/7/1 Maciej Gajewski<maciej.gajewski0@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 <mailto:maciej.gajewski0@gmail.com>
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 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

ian link <mailto:ian@ilink.io>
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 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.stehule@gmail.com <mailto:pavel.stehule@gmail.com>> wrote:

Hello

2013/7/1 ian link <ian@ilink.io <mailto:ian@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.stehule@gmail.com <mailto:pavel.stehule@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.gajewski0@gmail.com

<mailto:maciej.gajewski0@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

Attachments:

postbox-contact.jpgimage/jpeg; name=postbox-contact.jpg; x-apple-mail-type=stationeryDownload
compose-unknown-contact.jpgimage/jpeg; name=compose-unknown-contact.jpg; x-apple-mail-type=stationeryDownload
#19Maciej Gajewski
maciej.gajewski0@gmail.com
In reply to: Ian Link (#18)
1 attachment(s)
Re: Review: query result history in psql

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

Attachments:

psql-ans.3.diffapplication/octet-stream; name=psql-ans.3.diffDownload
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index 5b77173..1654607 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -20,7 +20,7 @@ REFDOCDIR= $(top_srcdir)/doc/src/sgml/ref
 
 override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) -I$(top_srcdir)/src/bin/pg_dump $(CPPFLAGS)
 
-OBJS=	command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
+OBJS=	ans.o command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
 	startup.o prompt.o variables.o large_obj.o print.o describe.o \
 	tab-complete.o mbprint.o dumputils.o keywords.o kwlookup.o \
 	sql_help.o \
diff --git a/src/bin/psql/ans.c b/src/bin/psql/ans.c
new file mode 100644
index 0000000..89d4c9a
--- /dev/null
+++ b/src/bin/psql/ans.c
@@ -0,0 +1,386 @@
+/*
+ * psql - the PostgreSQL interactive terminal
+ *
+ * Copyright (c) 2013, PostgreSQL Global Development Group
+ *
+ * src/bin/psql/ans.c
+ */
+#include "postgres_fe.h"
+
+#include "ans.h"
+#include "stringutils.h"
+
+#include "postgres_fe.h"
+
+static int global_ans_num = 0;
+
+static void CreateTable(PGconn *db, struct _ans* item);
+static char* GetTypeName(PGconn *db, Oid oid);
+static char* BuildData(PGresult* result);
+
+/*
+ * Creates empty entry, serving as list head
+ */
+AnsHistory CreateAnsHistory(void)
+{
+	struct _ans* head;
+	
+	head = pg_malloc(sizeof(struct _ans));
+	
+	head->numColumns = 0;
+	head->columnTypes = NULL;
+	head->columnNames = NULL;
+	head->data = NULL;
+	head->next = NULL;
+	head->tableName = NULL;
+	head->name = NULL;
+	
+	return head;
+}
+
+void
+AddToHistory(AnsHistory history, PGresult* result)
+{
+	struct _ans* item;
+	int numRows, numColumns;
+	int i;
+	char* data;
+	
+	if (!history || !result)
+		return;
+	
+	numColumns  = PQnfields(result);
+	numRows = PQntuples(result);
+	
+	if (numColumns <= 0 || numRows <= 0)
+		return;
+	
+	data = BuildData(result);
+	if (!data)
+		return;
+	
+	item = pg_malloc(sizeof(struct _ans));
+
+	/* read meta-data: column names and types */
+	item->numColumns = numColumns;
+	item->columnNames = pg_malloc(sizeof(char*) * item->numColumns);
+	item->columnTypes = pg_malloc(sizeof(Oid) * item->numColumns);
+	
+	for (i = 0; i < item->numColumns; i++)
+	{
+		char* name;
+		
+		item->columnTypes[i] = PQftype(result, i);
+		
+		name = PQfname(result, i);
+		item->columnNames[i] = pg_malloc(strlen(name) + 1);
+		strcpy(item->columnNames[i], name);
+	}
+	
+	item->data = data;
+	
+	/* Table creation deferred to when the ASN is actually used */
+	item->tableName = NULL;
+
+	/* name */
+	item->name = pg_malloc(10);
+	sprintf(item->name, "ans%d", global_ans_num++);
+	
+	printf(_("Query result stored as :%s\n"), item->name);
+	
+	item->next = history->next;
+	history->next = item;
+}
+
+const char*
+GetOrCreateTable(AnsHistory history, PGconn *db, const char* name)
+{
+	struct _ans* item;
+	
+	if (!history || !name)
+	{
+		return NULL;
+	}
+	
+	item = history->next;
+	
+	while (item)
+	{
+		if (strcmp(item->name, name) == 0)
+		{
+			if (item->tableName)
+				return item->tableName;
+			
+			CreateTable(db, item);
+			if (item->tableName)
+				return item->tableName;
+			else
+				return NULL;
+		}
+		item = item->next;
+	}
+	
+	return NULL;
+}
+
+static
+void 
+CreateTable(PGconn *db, struct _ans* item)
+{
+	char tableNameBuf[32];
+	char copyQuery[64];
+	const int qbufsize=2048;
+	char queryBuf[qbufsize];
+	char* queryPtr;
+	int len;
+	int i;
+	PGresult* qres;
+	
+	if (!item)
+		return;
+	
+	len = snprintf(tableNameBuf, 32, "_table_%s", item->name);
+	if (len >= 32)
+	{
+		return;
+	}
+	len = snprintf(copyQuery, 64, "COPY %s FROM STDIN", tableNameBuf);
+	if (len >= 64)
+	{
+		return;
+	}
+	
+	/* Build CREATE TABLE query */
+	queryPtr = queryBuf;
+	queryPtr += snprintf(queryPtr, qbufsize-(int)(queryPtr-queryBuf), "CREATE TEMP TABLE %s (\n", tableNameBuf);
+	
+	for (i = 0; i < item->numColumns; ++i)
+	{
+		char* typeName;
+		const char* format;
+		
+		typeName = GetTypeName(db, item->columnTypes[i]);
+		if (!typeName)
+			return;
+		
+		if (i == item->numColumns-1)
+			format = "%s %s\n";
+		else
+			format = "%s %s,\n";
+			
+		queryPtr += snprintf(queryPtr, qbufsize-(int)(queryPtr-queryBuf), format, item->columnNames[i], typeName);
+		free(typeName);
+		if (queryPtr >= queryBuf+qbufsize)
+		{
+			return; 
+		}
+	}
+	queryPtr += snprintf(queryPtr, qbufsize-(int)(queryPtr-queryBuf), ");");
+	if (queryPtr >= queryBuf+qbufsize)
+	{
+		return; 
+	}
+	
+	/* create and populate table */
+	PQclear(PQexec(db, "BEGIN"));
+	qres = PQexec(db, queryBuf);
+	if (PQresultStatus(qres) != PGRES_COMMAND_OK)
+	{
+		PQclear(qres);
+		PQclear(PQexec(db, "ROLLBACK"));
+		return;
+	}
+	PQclear(qres);
+
+	qres = PQexec(db, copyQuery);
+	if (PQresultStatus(qres) != PGRES_COPY_IN)
+	{
+		PQclear(qres);
+		PQclear(PQexec(db, "ROLLBACK"));
+		return;
+	}
+	PQclear(qres);
+	
+	PQputCopyData(db, item->data, strlen(item->data));
+	len = PQputCopyEnd(db, NULL);
+	if (len != 1)
+	{
+		PQclear(PQexec(db, "ROLLBACK"));
+		return;
+	}
+	qres = PQgetResult(db);
+	if (PQresultStatus(qres) != PGRES_COMMAND_OK)
+	{
+		PQclear(qres);
+		PQclear(PQexec(db, "ROLLBACK"));
+		return;
+	}
+	PQclear(qres);
+	PQclear(PQexec(db, "COMMIT"));
+	
+	item->tableName = pg_strdup(tableNameBuf);
+}
+
+/* free the typename after use */
+static
+char* 
+GetTypeName(PGconn *db, Oid oid)
+{
+	const int bufsize = 1024;
+	char queryBuf[bufsize];
+	int sres;
+	PGresult* qres;
+	char* typeName;
+	
+	sres = snprintf(queryBuf, bufsize, "SELECT typname FROM pg_type WHERE oid=%d;", oid);
+	if ( sres < 0 || sres > bufsize)
+		return NULL;
+	
+	qres = PQexec(db, queryBuf);
+	if (!qres)
+		return NULL;
+	
+	if (PQresultStatus(qres) != PGRES_TUPLES_OK)
+	{
+		PQclear(qres);
+		return NULL;
+	}
+	
+	if (PQntuples(qres) != 1 || PQnfields(qres) != 1)
+	{
+		PQclear(qres);
+		return NULL;
+	}
+
+	typeName = pg_strdup(PQgetvalue(qres, 0, 0));
+	PQclear(qres);
+	return typeName;
+}
+
+/* Convert query data into data buffer in COPY TEXT format.
+ * returns NULL on error
+ * caller owns the data
+ */
+static 
+char* 
+BuildData(PGresult* result)
+{
+	int cols, rows;
+	int bufferSize;
+	int r,c;
+	char* buffer;
+	char* dataPtr;
+	
+	if (!result)
+		return NULL;
+	
+	cols = PQnfields(result);
+	rows = PQntuples(result);
+	
+	/* first pass - measure the size */
+	bufferSize = 0;
+	for(r = 0; r < rows; r++)
+	{
+		for(c = 0; c < cols; c++)
+		{
+			if (PQgetisnull(result, r, c))
+			{
+				bufferSize += 2; /* 2 = size of null literal \N */
+			}
+			else
+			{
+				bufferSize += get_escaped_for_copy_len(PQgetvalue(result, r, c));
+			}
+			bufferSize +=1; /* column or row delimiter*/
+		}
+	}
+	bufferSize += 4; /* end-of-data marker \. + NULL terminiator */
+	
+	/* second pass = build the buffer */
+	buffer = pg_malloc(bufferSize);
+	dataPtr = buffer;
+	
+	for(r = 0; r < rows; r++)
+	{
+		for(c = 0; c < cols; c++)
+		{
+			if (PQgetisnull(result, r, c))
+			{
+				strcpy(dataPtr, "\\N");
+				dataPtr += 2;
+			}
+			else
+			{
+				char* value = PQgetvalue(result, r, c);
+				dataPtr = escape_for_copy(dataPtr, value);
+			}
+			if (c == cols-1)
+			{
+				strcpy(dataPtr, "\n");
+			}
+			else
+			{
+				strcpy(dataPtr, "\t");
+			}
+			dataPtr += 1;
+		}
+	}
+	strcpy(dataPtr, "\\.\n");
+	
+	return buffer;
+}
+
+void AnsClearTableNames(AnsHistory history)
+{
+	struct _ans* item = history;
+	
+	while(item->next)
+	{
+		item = item->next;
+		
+		if (item->tableName)
+		{
+			free(item->tableName);
+			item->tableName = NULL;
+		}
+	}	
+}
+
+void
+DestroyAnsHistory(PGconn *db, AnsHistory item)
+{
+	if (!item)
+		return;
+	
+	if (item->next)
+		DestroyAnsHistory(db, item->next);
+	
+	if (item->data)
+		free(item->data);
+	
+	if(item->name)
+		free(item->name);
+	
+	if (item->columnTypes)
+		free(item->columnTypes);
+	
+	if (item->numColumns && item->columnNames)
+	{
+		int i;
+		for(i = 0; i < item->numColumns; i++)
+			free(item->columnNames[i]);
+		
+		free(item->columnNames);
+	}
+	
+	if (item->tableName)
+	{
+		const int bufsize = 32;
+		char deleteQuery[bufsize];
+
+		snprintf(deleteQuery, bufsize, "DROP TABLE %s", item->tableName);
+		PQclear(PQexec(db, deleteQuery));
+	}
+	
+	free(item);
+}
diff --git a/src/bin/psql/ans.h b/src/bin/psql/ans.h
new file mode 100644
index 0000000..033630d
--- /dev/null
+++ b/src/bin/psql/ans.h
@@ -0,0 +1,48 @@
+/*
+ * psql - the PostgreSQL interactive terminal
+ *
+ * Copyright (c) 2013, PostgreSQL Global Development Group
+ *
+ * src/bin/psql/ans.h
+ */
+#ifndef ANS_H
+#define ANS_H
+
+#include "libpq-fe.h"
+
+
+/* Cache of last N query results, stored in a format ready to insert itno tempary table when needed */
+struct _ans 
+{
+	int		numColumns;
+	Oid		*columnTypes;
+	char	**columnNames;
+	char	*data;
+	
+	char	*name; // history item name
+	char	*tableName; // corresponding table in DB. NULL if not created yet
+	
+	struct _ans	*next;
+};
+
+typedef struct _ans* AnsHistory;
+
+AnsHistory	CreateAnsHistory(void);
+void		AddToHistory(AnsHistory history, PGresult* result);
+void		DestroyAnsHistory(PGconn *db, AnsHistory item);
+
+
+/*
+ * Takes variable name and checks wether it matches the name of existing history item.
+ * If none found, returns NULL.
+ * If found, but table not created yet, creates the table first.
+ */
+const char* GetOrCreateTable(AnsHistory history, PGconn *db, const char* name);
+
+/* Should be called when new connection is open.
+ * Because all the tables used by ANS are temporary and kept in current DB, thety are no longer valid
+ * after the client connect to new database
+ */
+void AnsClearTableNames(AnsHistory history);
+
+#endif
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 351e684..b5ff49c 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -40,6 +40,7 @@
 #include "pqexpbuffer.h"
 #include "dumputils.h"
 
+#include "ans.h"
 #include "common.h"
 #include "copy.h"
 #include "describe.h"
@@ -51,6 +52,7 @@
 #include "psqlscan.h"
 #include "settings.h"
 #include "variables.h"
+#include "ans.h"
 
 
 /* functions for use in this file */
@@ -205,6 +207,40 @@ exec_command(const char *cmd,
 			success = do_pset("format", "unaligned", &pset.popt, pset.quiet);
 	}
 
+	/* \ans - toggle query result history */
+	else if (strcmp(cmd, "ans") == 0)
+	{
+		char	 *opt = psql_scan_slash_option(scan_state,
+												 OT_NORMAL, NULL, false);
+
+		if (opt)
+			pset.ans_enabled = ParseVariableBool(opt);
+		else
+			pset.ans_enabled = !pset.ans_enabled;
+		
+		if (!pset.quiet)
+		{
+			if (pset.ans_enabled)
+				puts(_("Query result history is on."));
+			else
+			{
+				puts(_("Query result history is off."));
+			}
+		}
+		free(opt);
+	}
+	/* \ansclean - clear query result history */
+	else if (strcmp(cmd, "ansclean") == 0)
+	{
+		DestroyAnsHistory(pset.db, pset.ans);
+		pset.ans = CreateAnsHistory();
+		
+		if (!pset.quiet)
+		{
+			puts(_("Query result history cleaned."));
+		}
+	}
+
 	/* \C -- override table title (formerly change HTML caption) */
 	else if (strcmp(cmd, "C") == 0)
 	{
@@ -1697,6 +1733,7 @@ do_connect(char *dbname, char *user, char *host, char *port)
 	PQsetNoticeProcessor(n_conn, NoticeProcessor, NULL);
 	pset.db = n_conn;
 	SyncVariables();
+	AnsClearTableNames(pset.ans);
 	connection_warnings(false); /* Must be after SyncVariables */
 
 	/* Tell the user about the new connection */
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 3dea92c..579bf45 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -23,6 +23,7 @@
 #include "command.h"
 #include "copy.h"
 #include "mbprint.h"
+#include "ans.h"
 
 
 
@@ -950,7 +951,11 @@ SendQuery(const char *query)
 
 		/* but printing results isn't: */
 		if (OK && results)
+		{
 			OK = PrintQueryResults(results);
+			if (pset.ans_enabled)
+				AddToHistory(pset.ans, results);
+		}
 	}
 	else
 	{
@@ -1227,7 +1232,7 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
 		}
 
 		printQuery(results, &my_popt, pset.queryFout, pset.logfile);
-
+		
 		PQclear(results);
 
 		/* after the first result set, disallow header decoration */
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 379dead..729b97a 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -289,6 +289,11 @@ slashUsage(unsigned short int pager)
 					  "  \\lo_import FILE [COMMENT]\n"
 					  "  \\lo_list\n"
 					  "  \\lo_unlink LOBOID      large object operations\n"));
+	fprintf(output, "\n");
+
+	fprintf(output, _("Query result history\n"));
+	fprintf(output, _("  \\ans [on|off]          toggle query result history (currently %s)\n"), ON(pset.ans_enabled));
+	fprintf(output, _("  \\ansclean              clean query result history\n"));
 
 	ClosePager(output);
 }
diff --git a/src/bin/psql/psqlscan.l b/src/bin/psql/psqlscan.l
index d61387d..686fbcc 100644
--- a/src/bin/psql/psqlscan.l
+++ b/src/bin/psql/psqlscan.l
@@ -737,11 +737,24 @@ other			.
 					}
 					else
 					{
-						/*
-						 * if the variable doesn't exist we'll copy the
-						 * string as is
+						/* 
+						 * This could be ANS variable, check it.
 						 */
-						ECHO;
+						const char* tableName;
+						
+						tableName = GetOrCreateTable(pset.ans, pset.db, varname);
+						if (tableName)
+						{
+							push_new_buffer(tableName, varname);
+						}
+						else
+						{
+							/*
+							* if the variable doesn't exist we'll copy the
+							* string as is
+							*/
+							ECHO;
+						}
 					}
 
 					free(varname);
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index e78aa9a..62c9137 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -11,6 +11,7 @@
 
 #include "variables.h"
 #include "print.h"
+#include "ans.h"
 
 #define DEFAULT_FIELD_SEP "|"
 #define DEFAULT_RECORD_SEP "\n"
@@ -93,6 +94,9 @@ typedef struct _psqlSettings
 	FILE	   *logfile;		/* session log file handle */
 
 	VariableSpace vars;			/* "shell variable" repository */
+	
+	AnsHistory	ans;			/* query result (answer) history.*/
+	bool		ans_enabled;	/* local query result storage enabled */
 
 	/*
 	 * The remaining fields are set by assign hooks associated with entries in
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index b2264c9..d8104e2 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -147,6 +147,9 @@ main(int argc, char *argv[])
 	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
 	SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
 	SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
+	
+	pset.ans = CreateAnsHistory();
+	pset.ans_enabled = 0;
 
 	parse_psql_options(argc, argv, &options);
 
diff --git a/src/bin/psql/stringutils.c b/src/bin/psql/stringutils.c
index 99968a1..ec3f639 100644
--- a/src/bin/psql/stringutils.c
+++ b/src/bin/psql/stringutils.c
@@ -338,3 +338,75 @@ quote_if_needed(const char *source, const char *entails_quote,
 
 	return ret;
 }
+
+int get_escaped_for_copy_len(const char* c)
+{
+	int len = 0;
+	for(; *c; c++)
+	{
+		switch(*c)
+		{
+			case '\\':
+			case '\b':
+			case '\f':
+			case '\r':
+			case '\n':
+			case '\t':
+			case '\v':
+				len++;
+			default:
+				len++;
+		}
+	}
+
+	return len;
+}
+
+char* escape_for_copy(char* dst, const char* c)
+{
+	for(; *c; c++)
+	{
+		switch(*c)
+		{
+			case '\\':
+				*(dst++) = '\\';
+				*(dst++) = '\\';
+				break;
+				
+			case '\b':
+				*(dst++) = '\\';
+				*(dst++) = 'b';
+				break;
+				
+			case '\f':
+				*(dst++) = '\\';
+				*(dst++) = 'f';
+				break;
+				
+			case '\r':
+				*(dst++) = '\\';
+				*(dst++) = 'r';
+				break;
+				
+			case '\n':
+				*(dst++) = '\\';
+				*(dst++) = 'n';
+				break;
+				
+			case '\t':
+				*(dst++) = '\\';
+				*(dst++) = 't';
+				break;
+				
+			case '\v':
+				*(dst++) = '\\';
+				*(dst++) = 'v';
+				break;
+				
+			default:
+				*(dst++) = *c;
+		}
+	}
+	return dst;
+}
+
diff --git a/src/bin/psql/stringutils.h b/src/bin/psql/stringutils.h
index bb2a194..6590fb9 100644
--- a/src/bin/psql/stringutils.h
+++ b/src/bin/psql/stringutils.h
@@ -24,4 +24,13 @@ extern void strip_quotes(char *source, char quote, char escape, int encoding);
 extern char *quote_if_needed(const char *source, const char *entails_quote,
 				char quote, char escape, int encoding);
 
+
+/* Returns lenght of string after escaping for COPY TEXT*/
+extern int get_escaped_for_copy_len(const char* c);
+
+/* Copies c to dst, escaping according to COPY TEXT format. Retuns pointer past the last character copied.
+ * Assumes there is enough room in the dst buffer (use get_escaped_for_copy_len to calculate the size beforehand)
+ */
+extern char* escape_for_copy(char* dst, const char* c);
+
 #endif   /* STRINGUTILS_H */
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 8eb9f83..f63149d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -891,7 +891,7 @@ psql_completion(char *text, int start, int end)
 	};
 
 	static const char *const backslash_commands[] = {
-		"\\a", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
+		"\\a", "\\ans", "\\ansclean", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy", "\\copyright",
 		"\\d", "\\da", "\\db", "\\dc", "\\dC", "\\dd", "\\dD", "\\des", "\\det", "\\deu", "\\dew", "\\df",
 		"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
 		"\\dn", "\\do", "\\dp", "\\drds", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du",
#20Peter Eisentraut
peter_e@gmx.net
In reply to: Maciej Gajewski (#19)
Re: Review: query result history in psql

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

#21ian link
ian@ilink.io
In reply to: Peter Eisentraut (#20)
Re: Review: query result history in psql

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 <peter_e@gmx.net> wrote:

Show quoted text

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"?

#22Robert Haas
robertmhaas@gmail.com
In reply to: ian link (#21)
Re: Review: query result history in psql

On Tue, Jul 2, 2013 at 8:16 PM, ian link <ian@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

#23ian link
ian@ilink.io
In reply to: Robert Haas (#22)
Re: Review: query result history in psql

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 <robertmhaas@gmail.com> wrote:

Show quoted text

On Tue, Jul 2, 2013 at 8:16 PM, ian link <ian@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

#24Pavel Stehule
pavel.stehule@gmail.com
In reply to: Robert Haas (#22)
Re: Review: query result history in psql

2013/7/3 Robert Haas <robertmhaas@gmail.com>:

On Tue, Jul 2, 2013 at 8:16 PM, ian link <ian@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

#25Josh Berkus
josh@agliodbs.com
In reply to: ian link (#1)
Re: Review: query result history in psql

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