On Thu, 27 Jan 2022 at 06:58, Michael Paquier <mich...@paquier.xyz> wrote: > > 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.
Thanks. Yes, will look at that. -- Simon Riggs http://www.EnterpriseDB.com/