Re: [v15 beta] pg_upgrade failed if earlier executed with -c switch

tather => rather
is charge => in charge

On Mon, Jun 06, 2022 at 01:17:52PM +0900, Michael Paquier wrote:
> but something like %Y%m%d_%H%M%S.%03d (3-digit ms for last part) would

On Tue, Jun 07, 2022 at 08:30:47AM +0900, Michael Paquier wrote:
> I would reduce the format to be YYYYMMDDTHHMMSS_ms (we could also use

I think it's better with a dot (HHMMSS.ms) rather than underscore (HHMMSS_ms).

On Tue, Jun 07, 2022 at 11:42:37AM +0900, Michael Paquier wrote:
> + /* append milliseconds */
> + snprintf(timebuf, sizeof(timebuf), "%s_%03d",
> + timebuf, (int) (time.tv_usec / 1000));

> + with a timestamp formatted as per ISO 8601
> + (<literal>%Y%m%dT%H%M%S</literal>) appended by an underscore and
> + the timestamp's milliseconds, where all the generated files are stored.

The ISO timestamp can include milliseconds (or apparently fractional parts of
the "lowest-order" unit), so the "appended by" part doesn't need to be
explained here.

+ snprintf(timebuf, sizeof(timebuf), "%s_%03d",
+ timebuf, (int) (time.tv_usec / 1000));

Is it really allowed to sprintf a buffer onto itself ?
I can't find any existing cases doing that.

It seems useless in any case - you could instead
snprintf(timebuf+strlen(timebuf), or increment len+=snprintf()...

Or append the milliseconds here:

+ len = snprintf(log_opts.basedir, MAXPGPATH, "%s/%s", log_opts.rootdir,
+ timebuf);

--
Justin