psql's \r broken since e984ef5861d

Started by Julien Rouhaudover 8 years ago6 messages
#1Julien Rouhaud
julien.rouhaud@dalibo.com
1 attachment(s)

Hello,

Unless I miss something, \r isn't working anymore, since
exec_command_print() fallback to display previous_buf if query_buf has
been freed.

Trivial patch to fix issue (free both buffers in exec_command_reset())
attached.

Regards.

--
Julien Rouhaud

Attachments:

fix_psql_r.difftext/x-patch; name=fix_psql_r.diffDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 14c64208ca..4087532052 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -109,7 +109,7 @@ static backslashResult exec_command_prompt(PsqlScanState scan_state, bool active
 static backslashResult exec_command_pset(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_quit(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_reset(PsqlScanState scan_state, bool active_branch,
-				   PQExpBuffer query_buf);
+				   PQExpBuffer query_buf, PQExpBuffer previous_buf);
 static backslashResult exec_command_s(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_set(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_setenv(PsqlScanState scan_state, bool active_branch,
@@ -369,7 +369,7 @@ exec_command(const char *cmd,
 	else if (strcmp(cmd, "q") == 0 || strcmp(cmd, "quit") == 0)
 		status = exec_command_quit(scan_state, active_branch);
 	else if (strcmp(cmd, "r") == 0 || strcmp(cmd, "reset") == 0)
-		status = exec_command_reset(scan_state, active_branch, query_buf);
+		status = exec_command_reset(scan_state, active_branch, query_buf, previous_buf);
 	else if (strcmp(cmd, "s") == 0)
 		status = exec_command_s(scan_state, active_branch);
 	else if (strcmp(cmd, "set") == 0)
@@ -2060,11 +2060,12 @@ exec_command_quit(PsqlScanState scan_state, bool active_branch)
  */
 static backslashResult
 exec_command_reset(PsqlScanState scan_state, bool active_branch,
-				   PQExpBuffer query_buf)
+				   PQExpBuffer query_buf, PQExpBuffer previous_buf)
 {
 	if (active_branch)
 	{
 		resetPQExpBuffer(query_buf);
+		resetPQExpBuffer(previous_buf);
 		psql_scan_reset(scan_state);
 		if (!pset.quiet)
 			puts(_("Query buffer reset (cleared)."));
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Julien Rouhaud (#1)
Re: psql's \r broken since e984ef5861d

Julien Rouhaud <julien.rouhaud@dalibo.com> writes:

Unless I miss something, \r isn't working anymore,

Works for me. Please describe exactly what misbehavior you're seeing.
What libreadline or libedit version are you using?

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

#3Julien Rouhaud
julien.rouhaud@dalibo.com
In reply to: Tom Lane (#2)
Re: psql's \r broken since e984ef5861d

On 20/07/2017 03:34, Tom Lane wrote:

Julien Rouhaud <julien.rouhaud@dalibo.com> writes:

Unless I miss something, \r isn't working anymore,

Works for me. Please describe exactly what misbehavior you're seeing.
What libreadline or libedit version are you using?

I have libreadline 7.0_p3.

Here's a simple test case, last \p still show the query buffer:

psql -X postgres

postgres=# select version();
version

--------------------------------------------------------------------------------------------------------------------
PostgreSQL 10beta2@decb08ebdf on x86_64-pc-linux-gnu, compiled by gcc
(Gentoo 4.9.3 p1.5, pie-0.6.4) 4.9.3, 64-bit
(1 row)

postgres=# \p
select version();
postgres=# \r
Query buffer reset (cleared).
postgres=# \p
select version();

On a 9.6:

postgres=# select version();
version

------------------------------------------------------------------------------------------------------------------
PostgreSQL 9.6.3@3c017a545f on x86_64-pc-linux-gnu, compiled by gcc
(Gentoo 4.9.3 p1.5, pie-0.6.4) 4.9.3, 64-bit
(1 row)

postgres=# \p
select version();
postgres=# \r
Query buffer reset (cleared).
postgres=# \p
Query buffer is empty.

--
Julien Rouhaud

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Julien Rouhaud (#3)
Re: psql's \r broken since e984ef5861d

Julien Rouhaud <julien.rouhaud@dalibo.com> writes:

On 20/07/2017 03:34, Tom Lane wrote:

Works for me. Please describe exactly what misbehavior you're seeing.

Here's a simple test case, last \p still show the query buffer:

Ah. I don't feel like trawling the archives for the discussion right now,
but I believe this was an intentional change to make the behavior more
consistent. Prior versions did things weirdly differently depending on
whether you'd typed anything, eg modifying your example slightly:

regression=# select version();
version
----------------------------------------------------------------------------------------------------------
PostgreSQL 9.6.3 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18), 64-bit
(1 row)

regression=# \p
select version();
regression=# mistake
regression-# \r
Query buffer reset (cleared).
regression=# \p
select version();
regression=# \g
version
----------------------------------------------------------------------------------------------------------
PostgreSQL 9.6.3 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-18), 64-bit
(1 row)

I think we felt that throwing away the previous-query buffer
when we didn't have to was generally to be avoided, so we
wanted to standardize on this behavior not the other one.
Do you think differently?

I have some recollection that there were also cases where \p
would print something different than what \g would execute,
which of course is quite nasty.

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#4)
Re: psql's \r broken since e984ef5861d

I wrote:

Ah. I don't feel like trawling the archives for the discussion right now,
but I believe this was an intentional change to make the behavior more
consistent.

Oh ... a quick look in the commit log finds the relevant discussion:
/messages/by-id/9b4ea968-753f-4b5f-b46c-d7d3bf7c8f90@manitou-mail.org

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

#6Julien Rouhaud
julien.rouhaud@dalibo.com
In reply to: Tom Lane (#5)
Re: psql's \r broken since e984ef5861d

On 20/07/2017 04:24, Tom Lane wrote:

I wrote:

Ah. I don't feel like trawling the archives for the discussion right now,
but I believe this was an intentional change to make the behavior more
consistent.

Oh ... a quick look in the commit log finds the relevant discussion:
/messages/by-id/9b4ea968-753f-4b5f-b46c-d7d3bf7c8f90@manitou-mail.org

Oh I see. Thanks a lot, sorry for the noise.

--
Julien Rouhaud

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