Re: Confusing behavior of psql's \e

From: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jacob Champion <pchampion(at)vmware(dot)com>, "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-10 10:32:02
Message-ID: b12a2464a0cdf8919cb0fc5f767346cf2b721c81.camel@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 2021-03-10 at 00:13 -0500, Tom Lane wrote:
> I agree
> that trying to get rid of the race condition inherent in the existing
> file mtime test would be a good idea. However, I've got some
> portability-related gripes about how you are doing the latter:
>
> 1. There is no principled reason to assume that the epoch date is in the
> past. IIRC, Postgres' timestamp epoch of 2000-01-01 was in the future
> at the time we set it. More relevant to the immediate issue, I clearly
> recall a discussion at Red Hat in which one of the principal glibc
> maintainers (likely Ulrich Drepper, though I'm not quite sure) argued
> that 32-bit time_t could be used indefinitely by redefining the epoch
> forward by 2^32 seconds every often; which would require intervals of
> circa 68 years in which time_t was seen as a negative offset from a
> future epoch date, rather than an unsigned offset from a past date.
> Now, I thought he was nuts then and I still think that 32-bit hardware
> will be ancient history by 2038 ... but there may be systems that do it
> like that. glibc hates ABI breakage.
>
> 2. Putting an enormously old date on a file that was just created will
> greatly confuse onlookers, some of whom (such as backup or antivirus
> daemons) might not react pleasantly.
>
> Between #1 and #2, it's clearly worth the extra one or two lines of
> code to set the file dates to, say, "time(NULL) - 1", rather than
> assuming that zero is good enough.
>
> 3. I wonder about the portability of utime(2). I see that we are using
> it to cause updates of socket and lock file times, but our expectations
> for it there are rock-bottom low. I think that failing the edit command
> if utime() fails is an overreaction even with optimistic assumptions about
> its reliability. Doing that makes things strictly worse than simply not
> doing anything, because 99% of the time this refinement is unnecessary.
>
> In short, I think the relevant code ought to be more like
>
> else
> {
> struct utimbuf ut;
>
> ut.modtime = ut.actime = time(NULL) - 1;
> (void) utime(fname, &ut);
> }
>
> (plus some comments of course)
>
> regards, tom lane
>
> PS: I seem to recall that some Microsoft filesystems have 2-second
> resolution on file mod times, so maybe it needs to be "time(NULL) - 2"?

Thanks for taking a look!

Taken together, these arguments are convincing.

Done like that in the attached patch version 4.

Yours,
Laurenz Albe

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2021-03-10 10:33:38 Do we support upgrade of logical replication?
Previous Message Amit Kapila 2021-03-10 10:18:47 Re: [PATCH] Provide more information to filter_prepare