On 10/23/2014 11:41 AM, David Rowley wrote:
> I'm not a big fan of this. It seems quite strange to be using Assert in
> this way. I'd rather see any error just silently fall back
> on GetSystemTimeAsFileTime() instead of this.
That's fair. I'd like some visibility into it, but I don't think it's vital.
> I had originally assumed
> that you stuck the debug log in there so that people would have some
> sort of way of finding out if their system is
> using GetSystemTimePreciseAsFileTime() or GetSystemTimeAsFileTime()
No, that was never the goal. The previous code using elog only logged if
the system couldn't load GetSystemTimePreciseAsFileTime() because of an
error other than the expected one when the symbol can't be found.
In other words, if you're on win2k8 nothing happens, it just silently
uses GetSystemTimeAsFileTime(). We expect failure to load the proc
address, that's ok, we just assume it's an older windows. If the load
fails for some _other_ reason though, that's a weird issue that's worth
complaining about, but we don't know anything more than "something isn't
right here".
> if (pg_get_system_time == &GetSystemTimeAsFileTime)
> elog(DEBUG1, "gettimeofday is using GetSystemTimeAsFileTime()");
> else
> elog(DEBUG1, "gettimeofday is using GetSystemTimePreciseAsFileTime()");
>
> But perhaps it's not worth the trouble.
That's probably not really worth it; it's completey different to what
the prior code was doing anyway.
> Also if you decide to get rid of the elog, probably should also remove
> the include of elog.h that you've added.
Rather.
-- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services