Re: recovery_target_time = 'now' is not an error but still impractical setting

On 18 August 2017 at 07:30, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
> On Thu, Aug 17, 2017 at 6:24 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On 15 August 2017 at 15:37, Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me> wrote:
>>
>>> One thing I tried was a combination of recovery_target_action =
>>> 'shutdown' and recovery_target_time = 'now'. The result is surprising
>>
>> Indeed, special timestamp values were never considered in the design,
>> so I'm not surprised they don't work and have never been tested.
>
> We could always use a TRY/CATCH block and add an error in
> GetCurrentDateTime and GetCurrentTimeUsec if they are called out of a
> transaction context. Rather-safe-than-sorry.
>
>> Your suggestion of "furthest" is already the default behaviour.
>>
>> Why are you using 'now'? Why would you want to pick a randomly
>> selected end time?
>
> "now" is not much interesting, targets in the past are more, like
> 'yesterday'. This could create back an instance back to the beginning
> of the previous day, simplifying scripts creating recovery.conf a bit,
> even if that's not much simplification as we are talking about
> creating a timestamp string.

I can't see any value in allowing imprecise and effective random timestamps.

ISTM if we care, it would be better to simply exclude the 7 named
timestamps prior to their being sent, as in the attached patch.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services