Re: Bug fix for psql's meta-command \ev - Mailing list pgsql-hackers

From Aleksander Alekseev
Subject Re: Bug fix for psql's meta-command \ev
Date
Msg-id CAJ7c6TMnvwgDRU5KmcTf_Nqar0mhuc6Q0FEtPnoJkNos_wQAbA@mail.gmail.com
Whole thread Raw
In response to Re: Bug fix for psql's meta-command \ev  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Responses Re: Bug fix for psql's meta-command \ev
List pgsql-hackers
Hi,

I came across the patch since it was marked as "Needs review" (and
then I realized that I mistakenly opened the upcoming commit fest, not
the current one...).

> Good catch! I agree to this.
>
> > This problem can be resolved by resetting the query buffer on
> > error. You can see the attached source code. After that, it will
> > result in output like the following:
>
> While exec_command_ef_ev() currently preserves the existing content of
> the query buffer in case of certain failures, This behavior doesn't
> seem to be particularly significant, especially given that both \ef
> and \ev are intended to overwrite the query buffer on success.
>
> We have the option to fix get_create_object_cmd() and ensure
> exec_command_ef_ev() retains the existing content of the query buffer
> on failure. However, this approach seems like overly cumbersome. So
> I'm +1 to this approach.
>
> A comment might be necessary to clarify that we need to wipe out the
> query buffer because it could be overwritten with an incomplete query
> string due to certain failures.

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.

-- 
Best regards,
Aleksander Alekseev



pgsql-hackers by date:

Previous
From: Greg Sabino Mullane
Date:
Subject: Re: Make --help output fit within 80 columns per line
Next
From: Matthias van de Meent
Date:
Subject: Re: Improving btree performance through specializing by key shape, take 2