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
signature.asc
Description: PGP signature