Re: PITR: enhance getRecordTimestamp() - Mailing list pgsql-hackers

On Wed, Nov 03, 2021 at 04:59:04PM +0000, Simon Riggs wrote:
> Thanks. Fixed and rebased.

+       if (xact_info == XLOG_XACT_PREPARE)
+       {
+           if (recoveryTargetUseOriginTime)
+           {
+               xl_xact_prepare *xlrec = (xl_xact_prepare *) XLogRecGetData(record);
+               xl_xact_parsed_prepare parsed;
+
+               ParsePrepareRecord(XLogRecGetInfo(record),
+                                 xlrec,
+                                 &parsed);
+               *recordXtime = parsed.origin_timestamp;
+           }
+           else
+               *recordXtime = ((xl_xact_prepare *) XLogRecGetData(record))->prepared_at;

As I learnt recently with ece8c76, there are cases where an origin
timestamp may not be set in the WAL record that includes the origin
timestamp depending on the setup done on the origin cluster.  Isn't
this code going to finish by returning true when enabling
recovery_target_use_origin_time in some cases, even if recordXtime is
0?  So it seems to me that this is lacking some sanity checks if
recordXtime is 0.

Could you add some tests for this proposal?  This adds various PITR
scenarios that would be uncovered, and TAP should be able to cover
that.
--
Michael