Re: pg_rewind does not rewind diverging timelines

Hello

I know there's still ongoing discussion on the direction itself, but I
focused on just testing and looking at the latest patch, in case the
fix remains the same.

+ PG_CATCH();
+ {
+ ErrorData *edata = CopyErrorData();
+
+ FlushErrorState();
+ ereport(FATAL,
+ errmsg("invalid UUID in history file \"%s\"", path),
+ errdetail("%s", edata->message));
+ }

This is missing a MemoryContextSwitchTo before CopyErrorData, and
results in an assertion with debug builds.

+ memset(&entry->tluuid, 0, sizeof(pg_uuid_t));
+ if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
+ rewind_parse_uuid(uuid_str, &entry->tluuid);

This ignores the return value of rewind_parse_uuid, possibly writing
partial garbage to tluuid on incorrect input.

Also, it seems like that with this patch, pg rewind requires the
target's history file to be always there - is this an intended change?
If yes, then it should be at least mentioned somewhere.

On master:
exit 0
"source and target cluster are on the same timeline"
"no rewind required"

On patched rewind:
exit 1
error: could not open file
".../tgt/pg_wal/00000002.history" for reading: No such file or directory

writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
+ const pg_uuid_t *newTLUUID,
XLogRecPtr switchpoint, char *reason)

In case this is a bug that should be backported, wouldn't this be an ABI break?

+#endif /* !FRONTEND */
+
+extern pg_uuid_t *generate_uuidv7(uint64 unix_ts_ms, uint32 sub_ms);
+extern pg_uuid_t *generate_uuidv7_r(pg_uuid_t *uuid, uint64
unix_ts_ms, uint32 sub_ms);

Shouldn't these go before the endif?