| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, bt23yoshidar(at)oss(dot)nttdata(dot)com |
| Subject: | Re: Bug fix for psql's meta-command \ev |
| Date: | 2023-09-19 03:53:59 |
| Message-ID: | ZQkbVxL9fKZxMwVE@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| Attachment | Content-Type | Size |
|---|---|---|
| bug_fix_for_ev_command_v2.patch | text/x-diff | 482 bytes |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zhijie Hou (Fujitsu) | 2023-09-19 04:10:39 | Move global variables of pgoutput to plugin private scope. |
| Previous Message | Amit Kapila | 2023-09-19 03:06:35 | Re: Add 'worker_type' to pg_stat_subscription |