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?
