[Tom Lane <tgl@sss.pgh.pa.us>]

We had a complaint (see [1], but it's not the first IIRC) about how
psql doesn't behave very nicely if one ends \sf or allied commands
with a semicolon:

regression=# \sf sin(float8);
ERROR:  expected a right parenthesis

This is a bit of a usability gotcha, since many other backslash
commands are forgiving about trailing semicolons.  I looked at
the code and found that it's actually trying to ignore semicolons,
by passing semicolon = true to psql_scan_slash_option.  But that
fails to work because it's also passing type = OT_WHOLE_LINE,
and the whole-line code path ignores the semicolon flag.  Probably
that's just because nobody needed to use that combination back in
the day.  There's another user of OT_WHOLE_LINE, exec_command_help,
which also wants this behavior and has written its own stripping
code to get it.  That seems pretty silly, so here's a quick finger
exercise to move that logic into psql_scan_slash_option.

Is this enough of a bug to deserve back-patching?  I'm not sure.

			regards, tom lane

[1] https://www.postgresql.org/message-id/CAEs%3D6D%3DnwX2wm0hjkaw6C_LnqR%2BNFtnnzbSzeZq-xcfi_ooKSw%40ma...

Attachments:

ignore-trailing-semi-in-sf-ef-sv-ev.patch (text/x-diff, 3.2K) download
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9103bc3465..2f74bfdc96 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1640,18 +1640,7 @@ exec_command_help(PsqlScanState scan_state, bool active_branch)
 	if (active_branch)
 	{
 		char	   *opt = psql_scan_slash_option(scan_state,
-												 OT_WHOLE_LINE, NULL, false);
-		size_t		len;
-
-		/* strip any trailing spaces and semicolons */
-		if (opt)
-		{
-			len = strlen(opt);
-			while (len > 0 &&
-				   (isspace((unsigned char) opt[len - 1])
-					|| opt[len - 1] == ';'))
-				opt[--len] = '\0';
-		}
+												 OT_WHOLE_LINE, NULL, true);
 
 		helpSQL(opt, pset.popt.topt.pager);
 		free(opt);
@@ -3165,6 +3154,10 @@ ignore_slash_filepipe(PsqlScanState scan_state)
  * This *MUST* be used for inactive-branch processing of any slash command
  * that takes an OT_WHOLE_LINE option.  Otherwise we might consume a different
  * amount of option text in active and inactive cases.
+ *
+ * Note: although callers might pass "semicolon" as either true or false,
+ * we need not duplicate that here, since it doesn't affect the amount of
+ * input text consumed.
  */
 static void
 ignore_slash_whole_line(PsqlScanState scan_state)
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 514977e59d..10a35dffc4 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -18,6 +18,8 @@
  */
 #include "postgres_fe.h"
 
+#include <ctype.h>
+
 #include "common.h"
 #include "psqlscanslash.h"
 
@@ -640,7 +642,22 @@ psql_scan_slash_option(PsqlScanState state,
 			termPQExpBuffer(&mybuf);
 			return NULL;
 		case xslashwholeline:
-			/* always okay */
+			/*
+			 * In whole-line mode, we interpret semicolon = true as stripping
+			 * trailing whitespace as well as semi-colons; this gives the
+			 * nearest equivalent to what semicolon = true does in normal
+			 * mode.  Note there's no concept of quoting in this mode.
+			 */
+			if (semicolon)
+			{
+				while (mybuf.len > 0 &&
+					   (mybuf.data[mybuf.len - 1] == ';' ||
+						(isascii((unsigned char) mybuf.data[mybuf.len - 1]) &&
+						 isspace((unsigned char) mybuf.data[mybuf.len - 1]))))
+				{
+					mybuf.data[--mybuf.len] = '\0';
+				}
+			}
 			break;
 		default:
 			/* can't get here */
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 5d61e4c7bb..4f3fd46420 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -5323,7 +5323,7 @@ END
          LANGUAGE sql
          IMMUTABLE PARALLEL SAFE STRICT COST 1
 1       RETURN ($2 + $1)
-\sf ts_debug(text)
+\sf ts_debug(text);
 CREATE OR REPLACE FUNCTION pg_catalog.ts_debug(document text, OUT alias text, OUT description text, OUT token text, OUT dictionaries regdictionary[], OUT dictionary regdictionary, OUT lexemes text[])
  RETURNS SETOF record
  LANGUAGE sql
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index f199d624d3..c997106b9f 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1315,7 +1315,7 @@ drop role regress_psql_user;
 \sf information_schema._pg_index_position
 \sf+ information_schema._pg_index_position
 \sf+ interval_pl_time
-\sf ts_debug(text)
+\sf ts_debug(text);
 \sf+ ts_debug(text)
 
 -- AUTOCOMMIT