Alex Shulgin <a...@commandprompt.com> writes: > Petr Jelinek <p...@2ndquadrant.com> writes: >> >> I had a quick look, the patch does not apply cleanly anymore but it's >> just release notes so nothing too bad. > > Yes, there were some ongoing changes that touched some parts of this and > I must have missed the latest one (or maybe I was waiting for it to > settle down).
The rebased version is attached. >> - the StandbyModeRequested and StandbyMode should be lowercased like >> the rest of the GUCs > > Yes, except that standby_mode is linked to StandbyModeRequested, not the > other one. I can try see if there's a sane way out of this. As I see it, renaming these will only add noise to this patch, and there is also ArchiveRecoveryRequested / InArchiveRecovery. This is going to be tricky and I'm not sure it's really worth the effort. >> - The whole CopyErrorData and memory context logic is not needed in >> check_recovery_target_time() as the FlushErrorState() is not called >> there > > You must be right. I recall having some trouble with strings being > free'd prematurely, but if it's ERROR, then there should be no need for > that. I'll check again. Hrm, after going through this again I'm pretty sure that was correct: the only way to obtain the current error message is to use CopyErrorData(), but that requires you to switch back to non-error memory context (via Assert). The FlushErrorState() call is not there because it's handled by the hook caller (or it can exit via ereport() with elevel==ERROR). >> - The new code in StartupXLOG() like >> + if (recovery_target_xid_string != NULL) >> + InitRecoveryTarget(RECOVERY_TARGET_XID); >> + >> + if (recovery_target_time_string != NULL) >> + InitRecoveryTarget(RECOVERY_TARGET_TIME); >> + >> + if (recovery_target_name != NULL) >> + InitRecoveryTarget(RECOVERY_TARGET_NAME); >> + >> + if (recovery_target_string != NULL) >> + InitRecoveryTarget(RECOVERY_TARGET_IMMEDIATE); >> >> would probably be better in separate function that you then call >> StartupXLOG() as StartupXLOG() is crazy long already so I think it's >> preferable to not make it worse. > > We can move it at top of CheckStartingAsStandby() obviously. Moved. -- Alex
-- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers