Re: Confusing behavior of psql's \e

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Jacob Champion <pchampion(at)vmware(dot)com>
Cc: "chap(at)anastigmatix(dot)net" <chap(at)anastigmatix(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Confusing behavior of psql's \e
Date: 2021-03-04 16:41:21
Message-ID: ff3c62209a93c1a3987b0003c3ccdb5056977411.camel@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 2021-03-03 at 17:12 +0000, Jacob Champion wrote:
> On Wed, 2021-03-03 at 17:08 +0100, Laurenz Albe wrote:
> > This is no new behavior, and I think this is rare enough that we don't
> > have to bother.
>
> I agree that it's not new behavior, but this patch exposes that
> behavior for the temporary file case, because you've fixed the bug that
> caused the timestamp check to not matter. As far as I can tell, you
> can't run into that race on the master branch for temporary files, and
> you won't run into it in practice for explicit filenames.

Actually, the timestamp check *did* matter before.
The code in "do_edit" has:

[after the editor has been called]
if (!error && before.st_mtime != after.st_mtime)
{
[read file back into query_buf]
}

This is pre-existing code. I just added an else branch:

else
{
[discard query_buf if we were editing a script, function or view]
}

So if you do your "modify and save the file in less than a second"
trick with the old code, you would end up with the old, unmodified
data in the query buffer.

I would say that the old behavior is worse in that case, and
discarding the query buffer is better.

I am not against fixing or improving the behavior, but given the
fact that we can't portably get less than second precision, it
seems impossible. For good measure, I have added a check if the
file size has changed.

I don't think we can or have to do better than that.
Few people are skilled enough to modify and save a file in less
than a second, and I don't think there have been complaints
about that behavior so far.

Attached is version 2 of the patch, with the added file size
check and a pgindent run.

Yours,
Laurenz Albe

Attachment Content-Type Size
0001-Improve-e-ef-and-ev-if-the-editor-is-quit.v2.patch text/x-patch 6.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ibrar Ahmed 2021-03-04 16:41:52 Re: [HACKERS] PATCH: Batch/pipelining support for libpq
Previous Message Chapman Flack 2021-03-04 16:41:10 Re: [PATCH] regexp_positions ( string text, pattern text, flags text ) → setof int4range[]