Re: Bug fix for psql's meta-command \ev

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Aleksander Alekseev <aleksander@timescale.com>
Cc: pgsql-hackers@lists.postgresql.org, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, bt23yoshidar@oss.nttdata.com
Date: 2023-09-19T03:53:59Z
Lists: pgsql-hackers

Attachments

On Mon, Sep 18, 2023 at 06:54:50PM +0300, Aleksander Alekseev wrote:
> I tested the patch and it LGTM too. I don't have a strong opinion on
> whether we should bother with a comment or not.
> 
> As a side note I wonder whether we shouldn't assume that query_buf is
> always properly initialized elsewhere. But this is probably out of
> scope of this particular discussion.

The patch looks incorrect to me.  In case you've not noticed, we'd
still have the same problem if do_edit() fails for a reason or
another, and there are plenty of these in this code path, even if I
agree that all of them are very unlikely.  For example:
- Emulate a failure in do_edit(), any way is fine, like forcing a
return false at the beginning of the routine.
- Attempt \ev on a valid view.  This passes lookup_object_oid() and
get_create_object_cmd(), fails at do_edit while switching the status
to PSQL_CMD_ERROR.
- The query buffer is incorrect, a follow-up query still fails.

Adding a comment looks important to me once we consider the edit as a
path that can fail and the edited query is only executed then reset
when we have PSQL_CMD_NEWEDIT as status.  I would suggest the patch
attached instead, taking care of the error case of this thread and the
ones I've spotted.
--
Michael

Commits

  1. psql: Reset query buffer of \e, \ef and \ev on error