From: Tristan Partin <tristan@neon.tech>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: Make psql ignore trailing semicolons in \sf, \ef, etc
Date: Mon, 08 Jan 2024 17:31:37 -0600
Message-ID: <CY9PTB44QXST.3UGGRFU6HAYRX@neon.tech> (raw)
In-Reply-To: <2012251.1704746912@sss.pgh.pa.us>
References: <2012251.1704746912@sss.pgh.pa.us>
On Mon Jan 8, 2024 at 2:48 PM CST, Tom Lane wrote:
> 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...> + /*
> + * 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';
> + }
> + }
Seems like if there was going to be any sort of casting, it would be to
an int, which is what the man page says for these two function, though
isascii(3) explicitly mentions "unsigned char."
Small English nit-pick: I would drop the hyphen between semi and colons.
As for backpatching, seems useful in the sense that people can write the
same script for all supported version of Postgres using the relaxed
syntax.
--
Tristan Partin
Neon (https://neon.tech)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tristan@neon.tech, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: Make psql ignore trailing semicolons in \sf, \ef, etc
In-Reply-To: <CY9PTB44QXST.3UGGRFU6HAYRX@neon.tech>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox