Re: proposal: rounding up time value less than its unit.

On Thu, Sep 25, 2014 at 1:04 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> David Johnston <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> > On Thu, Sep 25, 2014 at 12:46 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> TBH I've also been wondering whether any of these proposed cures are
> >> better than the disease. The changes that can be argued to make the
> >> behavior more sane are also ones that introduce backwards compatibility
> >> issues of one magnitude or another. And I do not have a lot of sympathy
> >> for "let's not change anything except to throw an error in a case that
> >> seems ambiguous". That's mostly being pedantic, not helpful, especially
> >> seeing that the number of field complaints about it is indistinguishable
> >> from zero.
>
> > ​Then what does it matter that we'd choose to error-out?​
>
> The number of complaints about the *existing* behavior is indistinguishable
> from zero (AFAIR anyway). It does not follow that deciding to throw an
> error where we did not before will draw no complaints.
>
>
Any change has the potential to draw complaints. For you it seems that
"hey, I upgraded to 9.5 and my logs are being rotated out every minute
now. I thought I had that turned off" is the desired complaint. Greg
wants: "hey, my 1 hour log rotation is now happening every minute". If the
error message is written correctly most people upon seeing the error will
simply fix their configuration and move on - regardless of whether they
were proactive in doing so having read the release notes.

​And, so what if we get some complaints? We already disallow the specified
values (instead, turning them into zeros) so it isn't like we are further
restricting user capabilities.

If I was to commit a patch that didn't throw an error I'd ask the author to
provide an outline for each of the affected parameters and what it would
mean (if possible) for a setting currently rounded to zero to instead be
rounded to 1.

Its the same language in the release notes to get someone to avoid the
error as it is to get them to not be impacted by the rounding change so the
difference is those people who would not read the release notes. The
"error" outcome is simple, direct, and fool-proof - and conveys the fact
that what they are asking for is invalid. It may be a little scary but at
least we can be sure nothing bad is happening in the system. If the
argument is that there are two few possible instances out there to expend
the effort to catalog every parameter then there isn't enough surface area
to care about throwing an error. And I still maintain that anyone setting
values for a clean installation would rather be told their input is not
valid instead of silently making it so.

​Changing the unit ​for log_rotate_age when their is basically no one
asking for that seems like the worse solution at face value; my +1 not
withstanding. I gave that mostly because if you are going to overhaul the
system then making everything "ms" seems like the right thing to do. I
think that such an effort would be a waste of resources.

I don't have clients so maybe my acceptance of errors is overly optimistic
- but in this case I truly don't see enough people even realizing that
there was a change and those few that do should be able to read the error
and make the same change that they would need to make anyway if the
rounding option is chosen.

My only concern here, and it probably applies to any solution, is that the
change is implemented properly so that all possible user input areas throw
the error correctly and as appropriate to the provided interface. That is,
interactive use immediately errors out without changing the default values
while explicit/manual file changes cause the system to error at startup. I
haven't looked into the code parts of this but I have to imagine this is
already covered since even with units and such invalid input is still
possible and needs to be addressed; this only add one more check to that
existing routine.

David J.