[HACKERS] Trailing comma support in SELECT statements

2014-10-03 Thread Bogdan Pilch
Hi,
I have created a small patch to postgres source (in particular the
psql part of it) that accepts trailing comma at the end of list in
SELECT statement.

The idea is to be able to say both (with the same result):
SELECT a, b, c from t;
SELECT a, b, c, from t;

Attached you can find a patch containing regression test (incorporated
into the serial_schedule).
My patch is relative to origin/REL9_4_STABLE branch as that is the one
I started from.

My plea is to have this change merged into the main stream so that it
becomes available in upcoming releases.

This modification does not require any interaction with user.
It does not create any backward compatibility issues.
Not does it have any performance impact.

regards
bogdan
From 450c339b4284887782b30e154766a0ee90d6f7ee Mon Sep 17 00:00:00 2001
From: Bogdan Pilch bogdan.pi...@opensynergy.com
Date: Sat, 16 Aug 2014 19:42:29 +0200
Subject: [PATCH 1/3] BPI: Added support for ignoring the trailing comma in
 select statement

---
 src/backend/parser/gram.y | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7b9895d..345c6cb 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -12470,6 +12470,7 @@ ctext_row: '(' ctext_expr_list ')'	{ $$ = $2; }
  */
 
 opt_target_list: target_list		{ $$ = $1; }
+			| target_list ','{ $$ = $1; }
 			| /* EMPTY */			{ $$ = NIL; }
 		;
 
-- 
1.9.1


From 9faf5eec4975eb99ad7c8901e30742ba92c0c4cb Mon Sep 17 00:00:00 2001
From: Bogdan Pilch bogdan.pi...@opensynergy.com
Date: Sun, 28 Sep 2014 13:12:24 +0200
Subject: [PATCH 3/3] Added regression test for trailing comma select feature.

---
 .../regress/expected/select_trailing_comma.out | 53 ++
 src/test/regress/serial_schedule   |  1 +
 src/test/regress/sql/select_trailing_comma.sql | 16 +++
 3 files changed, 70 insertions(+)
 create mode 100644 src/test/regress/expected/select_trailing_comma.out
 create mode 100644 src/test/regress/sql/select_trailing_comma.sql

diff --git a/src/test/regress/expected/select_trailing_comma.out b/src/test/regress/expected/select_trailing_comma.out
new file mode 100644
index 000..f84938c
--- /dev/null
+++ b/src/test/regress/expected/select_trailing_comma.out
@@ -0,0 +1,53 @@
+--
+-- SELECT WITH TRAILING COMMA
+--
+CREATE TEMP TABLE primes (p1 int, p2 int, p3 int);
+INSERT INTO primes VALUES (13, 7927, 7);
+SELECT * FROM primes;
+ p1 |  p2  | p3 
++--+
+ 13 | 7927 |  7
+(1 row)
+
+SELECT *, FROM primes;
+ p1 |  p2  | p3 
++--+
+ 13 | 7927 |  7
+(1 row)
+
+SELECT p1 FROM primes;
+ p1 
+
+ 13
+(1 row)
+
+SELECT p1, FROM primes;
+ p1 
+
+ 13
+(1 row)
+
+SELECT p1, p2 FROM primes;
+ p1 |  p2  
++--
+ 13 | 7927
+(1 row)
+
+SELECT p1, p2, FROM primes;
+ p1 |  p2  
++--
+ 13 | 7927
+(1 row)
+
+SELECT p1, p2, p3 FROM primes;
+ p1 |  p2  | p3 
++--+
+ 13 | 7927 |  7
+(1 row)
+
+SELECT p1, p2, p3, FROM primes;
+ p1 |  p2  | p3 
++--+
+ 13 | 7927 |  7
+(1 row)
+
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 16a1905..3571d14 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -79,6 +79,7 @@ test: select_distinct
 test: select_distinct_on
 test: select_implicit
 test: select_having
+test: select_trailing_comma
 test: subselect
 test: union
 test: case
diff --git a/src/test/regress/sql/select_trailing_comma.sql b/src/test/regress/sql/select_trailing_comma.sql
new file mode 100644
index 000..a2c922f
--- /dev/null
+++ b/src/test/regress/sql/select_trailing_comma.sql
@@ -0,0 +1,16 @@
+--
+-- SELECT WITH TRAILING COMMA
+--
+
+CREATE TEMP TABLE primes (p1 int, p2 int, p3 int);
+
+INSERT INTO primes VALUES (13, 7927, 7);
+
+SELECT * FROM primes;
+SELECT *, FROM primes;
+SELECT p1 FROM primes;
+SELECT p1, FROM primes;
+SELECT p1, p2 FROM primes;
+SELECT p1, p2, FROM primes;
+SELECT p1, p2, p3 FROM primes;
+SELECT p1, p2, p3, FROM primes;
-- 
1.9.1


-- 
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] Time measurement format - more human readable

2014-10-02 Thread Bogdan Pilch
 On 9/29/14, 1:08 AM, Andres Freund wrote:
 On 2014-09-28 20:32:30 -0400, Gregory Smith wrote:
 There are already a wide range of human readable time interval output
 formats available in the database; see the list at 
 http://www.postgresql.org/docs/current/static/datatype-datetime.html#INTERVAL-STYLE-OUTPUT-TABLE
 He's talking about psql's \timing...
 
 I got that.  My point was that even though psql's timing report is
 kind of a quick thing hacked into there, if it were revised I'd
 expect two things will happen eventually:
 
 -Asking if any of the the interval conversion code can be re-used
 for this purpose, rather than adding yet another custom to one code
 path standard.
 
 -Asking if this should really just be treated like a full interval
 instead, and then overlapping with a significant amount of that
 baggage so that you have all the existing format choices.

That's actually a good idea.
So what you're sayig is that if I come up with some nice way of
setting customized time output format, keeping the default the way it
is now, then it would be worth considering?

Now I understand why it says that a discussion is recommended before
implementing and posting. ;-)

bogdan


-- 
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] Time measurement format - more human readable

2014-09-30 Thread Bogdan Pilch
How about, the format of psql duration can be set via some ...
backslash command or commdn line switch? And the default of course
remains the current behavior?

bogdan
 Andres Freund and...@2ndquadrant.com writes:
  On 2014-09-28 20:32:30 -0400, Gregory Smith wrote:
  On 9/28/14, 7:49 AM, Bogdan Pilch wrote:
  I have created a small patch to postgres source (in particular the
  psql part of it) that modifies the way time spent executing the SQL
  commands is printed out.
 
  There are already a wide range of human readable time interval output
  formats available in the database; see the list at 
  http://www.postgresql.org/docs/current/static/datatype-datetime.html#INTERVAL-STYLE-OUTPUT-TABLE
 
  He's talking about psql's \timing...
 
 Indeed.  Still, it seems like this has more downside than upside.
 It seems likely to break some peoples' scripts, and where exactly
 is the groundswell of complaint that the existing format is
 unreadable?  TBH, I've not heard even one complaint about that
 before today.  On the other hand, the number of complaints we will
 get if we change the format is likely to be more than zero.
 
   regards, tom lane


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


[HACKERS] Time measurement format - more human readable

2014-09-28 Thread Bogdan Pilch
Hi,
I have created a small patch to postgres source (in particular the
psql part of it) that modifies the way time spent executing the SQL
commands is printed out.

The idea is to have a human readable time printed, e.g.:
Time: 1:32:15.45 m:s:ms
Time: 2_10:12:55:444.033 d_h:m:s:ms

Attached you can find a patch without any regression tests for that as
this is practically impossible to test with regression tests. The
duration of an SQL command (even though using pg_sleep) would differ
on each machine and even between consecutive runs. Therefore one
cannot specify a static expected output.
My patch is relative to origin/REL9_4_STABLE branch as that is the one
I started from.

My plea is to have this change merged into the main stream so that it
becomes available in upcoming releases.

This modification does not require any interaction with user.
It may create backward compatibility issues if some SQL developers
assumed that the format is always milis.micros.

regards
bogdan

From 25b2e3f9d888ecf0cc6fe0fbb569004cf9ce315b Mon Sep 17 00:00:00 2001
From: Bogdan Pilch bogdan.pi...@opensynergy.com
Date: Sat, 16 Aug 2014 23:20:18 +0200
Subject: [PATCH] BPI: Implemented enhancement f timing feature (displaying
 time in a more readable way).

---
 src/bin/psql/common.c | 75 +--
 1 file changed, 73 insertions(+), 2 deletions(-)

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index c08c813..9c7f1ff 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -25,10 +25,18 @@
 #include mbprint.h
 
 
+#define TIMING_BUFFER_SIZE 64
+
+#define SECONDS_DENOMINATOR (1000.0)
+#define MINUTES_DENOMINATOR (60.0 * SECONDS_DENOMINATOR)
+#define HOURS_DENOMINATOR (60.0 * MINUTES_DENOMINATOR)
+#define DAYS_DENOMINATOR (24.0 * HOURS_DENOMINATOR)
+
 
 static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
 static bool command_no_begin(const char *query);
 static bool is_select_command(const char *query);
+void ms2str_format(double intime, char *out);
 
 /*
  * setQFout
@@ -880,6 +888,7 @@ SendQuery(const char *query)
 	bool		OK = false;
 	bool		on_error_rollback_savepoint = false;
 	static bool on_error_rollback_warning = false;
+	char timing_buf[TIMING_BUFFER_SIZE];
 
 	if (!pset.db)
 	{
@@ -1063,8 +1072,11 @@ SendQuery(const char *query)
 	PQclear(results);
 
 	/* Possible microtiming output */
-	if (pset.timing)
-		printf(_(Time: %.3f ms\n), elapsed_msec);
+	if (pset.timing) {
+		ms2str_format(elapsed_msec, timing_buf);
+		//printf(_(Time: %.3f ms\n), elapsed_msec);
+		printf(_(Time: %s\n), timing_buf);
+	}
 
 	/* check for events that may occur during query execution */
 
@@ -1748,3 +1760,62 @@ expand_tilde(char **filename)
 
 	return;
 }
+
+/*
+ * Fill in the supplied buffer with nice time broken down to dd:hh:mm:ss:ms.us
+ *
+ */
+void ms2str_format(double intime, char *out)
+{
+	int days, hours, minutes, seconds;
+	double ms;
+
+	days = (int) (intime / (DAYS_DENOMINATOR));
+	intime -= ((double)days * DAYS_DENOMINATOR);
+	hours = (int) (intime / (HOURS_DENOMINATOR));
+	intime -= ((double)hours * HOURS_DENOMINATOR);
+	minutes = (int) (intime / (MINUTES_DENOMINATOR));
+	intime -= ((double)minutes * MINUTES_DENOMINATOR);
+	seconds = (int) (intime / (SECONDS_DENOMINATOR));
+	intime -= ((double)seconds * SECONDS_DENOMINATOR);
+	ms = (intime);
+
+	if (days  0)
+		sprintf(out, %d_%d:%d:%d:%.02f d_h:m:s:ms, days, hours, minutes, seconds, ms);
+	else if (hours  0)
+		sprintf(out, %d:%d:%d:%.02f h:m:s:ms, hours, minutes, seconds, ms);
+	else if (minutes  0)
+		sprintf(out, %d:%d:%.02f m:s:ms, minutes, seconds, ms);
+	else if (seconds  0)
+		sprintf(out, %d:%.02f s:ms, seconds, ms);
+	else
+		sprintf(out, %.02f ms, ms);
+}
-- 
1.9.1


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


[HACKERS] Tab expansion - on/off feature

2014-09-28 Thread Bogdan Pilch
Hi,
I have created a small patch to postgres source (in particular the
psql part of it) that modifies the way tab expansion is handled.

The idea is to be able to toggle tab expansion, having the default set
to ON (as it is now). If turned off, tab characters on command line in
interactive mode are not evaluated nor expanded, but just copied.

Tab expansion can either be turned off using command line option (-C) or
controlled by \tab internal command of psql.

Attached you can find a patch. I haven't created any regression tests
as tab expansion works only in interactive mode.
My patch is relative to origin/REL9_4_STABLE branch as that is the one
I started from.

My plea is to have this change merged into the main stream so that it
becomes available in upcoming releases.

This modification introduces new (optional) command line option and a
new internal backslash command.
It does not create any backward compatibility issues as the default
behavior remains unchanged.

regards
bogdan

From e3ba6cda83b64246c2b4d3df01f62444f4b37c9d Mon Sep 17 00:00:00 2001
From: Bogdan Pilch bogdan.pi...@opensynergy.com
Date: Sun, 7 Sep 2014 18:59:12 +0200
Subject: [PATCH] Implemented support for turning off/on tab completion (in
 readline).

---
 src/bin/psql/command.c  | 20 
 src/bin/psql/help.c |  1 +
 src/bin/psql/input.c|  9 +
 src/bin/psql/input.h|  1 +
 src/bin/psql/mainloop.c |  3 +++
 src/bin/psql/settings.h |  1 +
 src/bin/psql/startup.c  |  7 ++-
 src/bin/psql/tab-complete.c |  9 +
 src/bin/psql/tab-complete.h |  1 +
 9 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 741a72d..e14b15b 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1343,6 +1343,26 @@ exec_command(const char *cmd,
 		free(value);
 	}
 
+	/* \tab -- toggle tab completion */
+	else if (strcmp(cmd, tab) == 0)
+	{
+		char	   *opt = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+		if (opt)
+			pset.tab_completion = ParseVariableBool(opt);
+		else
+			pset.tab_completion = !pset.tab_completion;
+		if (!pset.quiet)
+		{
+			if (pset.tab_completion)
+puts(_(Tab completion is on.));
+			else
+puts(_(Tab completion is off.));
+		}
+		free(opt);
+	}
+
 	/* \timing -- toggle timing of queries */
 	else if (strcmp(cmd, timing) == 0)
 	{
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 3aa3c16..afc90b8 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -87,6 +87,7 @@ usage(void)
 
 	printf(_(\nInput and output options:\n));
 	printf(_(  -a, --echo-all   echo all input from script\n));
+	printf(_(  -C, --tab-completion-off turn off tab completion\n));
 	printf(_(  -e, --echo-queries   echo commands sent to server\n));
 	printf(_(  -E, --echo-hiddendisplay queries that internal commands generate\n));
 	printf(_(  -L, --log-file=FILENAME  send session log to file\n));
diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index aa32a3f..96f73c6 100644
--- a/src/bin/psql/input.c
+++ b/src/bin/psql/input.c
@@ -263,6 +263,15 @@ decode_history(void)
 
 
 /*
+ * Just a wrapper function for readline setup
+ */
+void
+set_input_completion(void)
+{
+	set_readline_completion();
+}
+
+/*
  * Put any startup stuff related to input in here. It's good to maintain
  * abstraction this way.
  *
diff --git a/src/bin/psql/input.h b/src/bin/psql/input.h
index 1d10a22..ad1eede 100644
--- a/src/bin/psql/input.h
+++ b/src/bin/psql/input.h
@@ -41,6 +41,7 @@
 char	   *gets_interactive(const char *prompt);
 char	   *gets_fromFile(FILE *source);
 
+void		set_input_completion(void);
 void		initializeInput(int flags);
 bool		saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag);
 
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index c3aff20..92546e0 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -123,6 +123,9 @@ MainLoop(FILE *source)
 
 		fflush(stdout);
 
+		/* Modify readline settings if necessary */
+		set_input_completion();
+
 		/*
 		 * get another line
 		 */
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 0a60e68..7e5e98c 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -89,6 +89,7 @@ typedef struct _psqlSettings
 	uint64		lineno;			/* also for error reporting */
 
 	bool		timing;			/* enable timing of all queries */
+	bool		tab_completion;		/* enable/disable tab completion in interactive mode */
 
 	FILE	   *logfile;		/* session log file handle */
 
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 45653a1..df9d720 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -121,6 +121,7 @@ main(int argc, char *argv[])
 	pset.copyStream = NULL;
 	pset.cur_cmd_source = stdin;
 	pset.cur_cmd_interactive = false;
+	pset.tab_completion = true;
 
 	/* We rely on unmentioned fields of pset.popt to start out 0/false/NULL