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?