On 2014-08-21 11:43:48 +0900, Sawada Masahiko wrote:
> On Wed, Aug 20, 2014 at 9:00 PM, Jeevan Chalke
> <jeevan.cha...@enterprisedb.com> wrote:
> > Hi,
> >
> > I have reviewed this:
> >
> > I have initialize cur_lineno to UINTMAX - 2. And then observed following
> > behaviour to check wrap-around.
> >
> > postgres=# \set PROMPT1 '%/[%l]%R%# '
> > postgres[18446744073709551613]=# \set PROMPT2 '%/[%l]%R%# '
> > postgres[18446744073709551613]=# select
> > postgres[18446744073709551614]-# a
> > postgres[18446744073709551615]-# ,
> > postgres[0]-# b
> > postgres[1]-# from
> > postgres[2]-# dual;
> >
> > It is wrapping to 0, where as line number always start with 1. Any issues?
> >
> > I would like to ignore this as UINTMAX lines are too much for a input
> > buffer to hold. It is almost NIL chances to hit this.
> >
> >
> > However, I think you need to use UINT64_FORMAT while printing uint64
> > number. Currently it is %u which wrap-around at UINT_MAX.
> > See how pset.lineno is displayed.
> >
> > Apart from this, I didn't see any issues in my testing.
> >
> > Patch applies cleanly. make/make install/initdb/make check all are well.
> >
> 
> Thank you for reviewing the patch!
> Attached patch is latest version patch.
> I modified the output format of cur_lineno.

I like the feature - and I wanted to commit it, but enough stuff turned
up that I needed to fix that it warrants some new testing.

Stuff I've changed:
* removed include of limits.h - that probably was a rememnant from a
  previous version
* removed a trailing whitespace
* expanded the documentation about %l. "The current line number" isn't
  very clear. Of a file? Of all lines ever entered in psql? It's now
  "The line number inside the current statement, starting from
  <literal>1</>."
* Correspondingly I've changed the variable's name to stmt_lineno.
* COPY FROM ... STDIN/PROMPT3 was broken because a) the promp was only generated
  once b) the lineno wasn't incremented.
* CTRL-C didn't reset the line number.
*  Unfortunately I've notice here that the prompting is broken in some
common cases:

postgres[1]=# SELECT 1,
postgres[2]-# '2
postgres[2]'# 2b
postgres[2]'# 2c
postgres[2]'# 2d',
postgres[3]-# 3;
┌──────────┬──────────┬──────────┐
│ ?column? │ ?column? │ ?column? │
├──────────┼──────────┼──────────┤
│        1 │ 2       ↵│        3 │
│          │ 2b      ↵│          │
│          │ 2c      ↵│          │
│          │ 2d       │          │
└──────────┴──────────┴──────────┘
(1 row)

postgres[1]=# SELECT 1,
'2
2b
2c
2d',
3
postgres[7]-#

That's rather inconsistent...


I've attached my version of the patch. Note that I've got rid of all the
PSCAN_INCOMPLETE checks... Please test!

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 476d799d74f2ea8eefc3480f176b3726c35cf425 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 26 Aug 2014 13:35:49 +0200
Subject: [PATCH] Add psql PROMPT variable showing which line of a statement is
 being edited.

The new %l substitution shows the line number inside a (potentially
multi-line) statement starting from one.

Author: Sawada Masahiko, editorialized by me.
Reviewed-By: Jeevan Chalke, Alvaro Herrera
---
 doc/src/sgml/ref/psql-ref.sgml |  9 +++++++++
 src/bin/psql/copy.c            | 15 +++++++++------
 src/bin/psql/mainloop.c        | 17 +++++++++++++++++
 src/bin/psql/prompt.c          |  5 +++++
 src/bin/psql/settings.h        |  1 +
 5 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 74d4618..db314c3 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -3316,6 +3316,15 @@ testdb=&gt; <userinput>INSERT INTO my_table VALUES (:'content');</userinput>
       </varlistentry>
 
       <varlistentry>
+        <term><literal>%l</literal></term>
+        <listitem>
+         <para>
+          The line number inside the current statement, starting from <literal>1</>.
+         </para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><literal>%</literal><replaceable class="parameter">digits</replaceable></term>
         <listitem>
         <para>
diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c
index c759abf..6908742 100644
--- a/src/bin/psql/copy.c
+++ b/src/bin/psql/copy.c
@@ -517,8 +517,8 @@ bool
 handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res)
 {
 	bool		OK;
-	const char *prompt;
 	char		buf[COPYBUFSIZ];
+	bool		showprompt = false;
 
 	/*
 	 * Establish longjmp destination for exiting from wait-for-input. (This is
@@ -540,21 +540,20 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res)
 	/* Prompt if interactive input */
 	if (isatty(fileno(copystream)))
 	{
+		showprompt = true;
 		if (!pset.quiet)
 			puts(_("Enter data to be copied followed by a newline.\n"
 				   "End with a backslash and a period on a line by itself."));
-		prompt = get_prompt(PROMPT_COPY);
 	}
-	else
-		prompt = NULL;
 
 	OK = true;
 
 	if (isbinary)
 	{
 		/* interactive input probably silly, but give one prompt anyway */
-		if (prompt)
+		if (showprompt)
 		{
+			const char *prompt = get_prompt(PROMPT_COPY);
 			fputs(prompt, stdout);
 			fflush(stdout);
 		}
@@ -589,8 +588,9 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res)
 			bool		firstload;
 			bool		linedone;
 
-			if (prompt)
+			if (showprompt)
 			{
+				const char *prompt = get_prompt(PROMPT_COPY);
 				fputs(prompt, stdout);
 				fflush(stdout);
 			}
@@ -650,7 +650,10 @@ handleCopyIn(PGconn *conn, FILE *copystream, bool isbinary, PGresult **res)
 			}
 
 			if (copystream == pset.cur_cmd_source)
+			{
 				pset.lineno++;
+				pset.stmt_lineno++;
+			}
 		}
 	}
 
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index c3aff20..9751009 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -58,6 +58,7 @@ MainLoop(FILE *source)
 	pset.cur_cmd_source = source;
 	pset.cur_cmd_interactive = ((source == stdin) && !pset.notty);
 	pset.lineno = 0;
+	pset.stmt_lineno = 1;
 
 	/* Create working state */
 	scan_state = psql_scan_create();
@@ -110,6 +111,7 @@ MainLoop(FILE *source)
 			count_eof = 0;
 			slashCmdStatus = PSQL_CMD_UNKNOWN;
 			prompt_status = PROMPT_READY;
+			pset.stmt_lineno = 1;
 			cancel_pressed = false;
 
 			if (pset.cur_cmd_interactive)
@@ -168,6 +170,7 @@ MainLoop(FILE *source)
 		count_eof = 0;
 
 		pset.lineno++;
+		pset.stmt_lineno++;
 
 		/* ignore UTF-8 Unicode byte-order mark */
 		if (pset.lineno == 1 && pset.encoding == PG_UTF8 && strncmp(line, "\xef\xbb\xbf", 3) == 0)
@@ -225,6 +228,18 @@ MainLoop(FILE *source)
 		{
 			PsqlScanResult scan_result;
 			promptStatus_t prompt_tmp = prompt_status;
+			char *tmp_line = line;
+
+			/*
+			 * Increase statement line number counter for each complete lines
+			 * already present in the line buffer. That's normally just the
+			 * case when navigating to a statement in the readline history.
+			 */
+			while (*tmp_line != '\0')
+			{
+				if (*(tmp_line++) == '\n')
+					pset.stmt_lineno++;
+			}
 
 			scan_result = psql_scan(scan_state, query_buf, &prompt_tmp);
 			prompt_status = prompt_tmp;
@@ -256,6 +271,7 @@ MainLoop(FILE *source)
 				/* execute query */
 				success = SendQuery(query_buf->data);
 				slashCmdStatus = success ? PSQL_CMD_SEND : PSQL_CMD_ERROR;
+				pset.stmt_lineno = 1;
 
 				/* transfer query to previous_buf by pointer-swapping */
 				{
@@ -303,6 +319,7 @@ MainLoop(FILE *source)
 												 query_buf : previous_buf);
 
 				success = slashCmdStatus != PSQL_CMD_ERROR;
+				pset.stmt_lineno = 1;
 
 				if ((slashCmdStatus == PSQL_CMD_SEND || slashCmdStatus == PSQL_CMD_NEWEDIT) &&
 					query_buf->len == 0)
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 26fca04..f2db9a9 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -44,6 +44,7 @@
  *		in prompt2 -, *, ', or ";
  *		in prompt3 nothing
  * %x - transaction status: empty, *, !, ? (unknown or no connection)
+ * %l - The line number inside the current statement, starting from 1.
  * %? - the error code of the last query (not yet implemented)
  * %% - a percent sign
  *
@@ -229,6 +230,10 @@ get_prompt(promptStatus_t status)
 						}
 					break;
 
+				case 'l':
+					snprintf(buf, sizeof(buf), UINT64_FORMAT, pset.stmt_lineno);
+					break;
+
 				case '?':
 					/* not here yet */
 					break;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 453d6c8..ef24a4e 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -88,6 +88,7 @@ typedef struct _psqlSettings
 	const char *progname;		/* in case you renamed psql */
 	char	   *inputfile;		/* file being currently processed, if any */
 	uint64		lineno;			/* also for error reporting */
+	uint64		stmt_lineno;	/* line number inside the current statement */
 
 	bool		timing;			/* enable timing of all queries */
 
-- 
2.0.0.rc2.4.g1dc51c6.dirty

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

Reply via email to