Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8; format=Flowed
Date: Mon, 08 Jan 2024 17:31:37 -0600
Message-Id: <CY9PTB44QXST.3UGGRFU6HAYRX@neon.tech>
From: "Tristan Partin" <tristan@neon.tech>
Subject: Re: Make psql ignore trailing semicolons in \sf, \ef, etc
To: "Tom Lane" <tgl@sss.pgh.pa.us>, <pgsql-hackers@lists.postgresql.org>
References: <2012251.1704746912@sss.pgh.pa.us>
In-Reply-To: <2012251.1704746912@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/CY9PTB44QXST.3UGGRFU6HAYRX%40neon.tech>
Precedence: bulk

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=3D# \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 =3D true to psql_scan_slash_option.  But that
> fails to work because it's also passing type =3D 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%40mail.gmail.com

>  +                        /*
>  +                         * In whole-line mode, we interpret semicolon =
=3D true as stripping
>  +                         * trailing whitespace as well as semi-colons; =
this gives the
>  +                         * nearest equivalent to what semicolon =3D tru=
e 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] =
=3D=3D ';' ||
>  +                                                (isascii((unsigned char=
) mybuf.data[mybuf.len - 1]) &&
>  +                                                 isspace((unsigned char=
) mybuf.data[mybuf.len - 1]))))
>  +                                {
>  +                                        mybuf.data[--mybuf.len] =3D '\0=
';
>  +                                }
>  +                        }

Seems like if there was going to be any sort of casting, it would be to=20
an int, which is what the man page says for these two function, though=20
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=20
same script for all supported version of Postgres using the relaxed=20
syntax.

--=20
Tristan Partin
Neon (https://neon.tech)