proposal: rounding up time value less than its unit.

On Thu, Jul 10, 2014 at 2:52 AM, Tomonari Katsumata
<katsumata.tomonari@po.ntts.co.jp> wrote:

Several couple weeks ago, I posted a mail to pgsql-doc.
/messages/by-id/53992FF8.2060702@po.ntts.co.jp

In this thread, I concluded that it's better to
round up the value which is less than its unit.
Current behavior (rounding down) has undesirable setting risk,
because some GUCs have special meaning for 0.

And then I made a patch for this.
Please check the attached patch.

Thanks for the patch. Please add it here:

https://commitfest.postgresql.org/action/commitfest_view/open

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Robert,

Thank you for checking this!

I've added it to commitfest.
https://commitfest.postgresql.org/action/patch_view?id=1507

regards,
------------
Tomonari Katsumata

2014-07-12 6:07 GMT+09:00 Robert Haas <robertmhaas@gmail.com>:

On 07/10/2014 09:52 AM, Tomonari Katsumata wrote:

Hi,

Several couple weeks ago, I posted a mail to pgsql-doc.
/messages/by-id/53992FF8.2060702@po.ntts.co.jp

In this thread, I concluded that it's better to
round up the value which is less than its unit.
Current behavior (rounding down) has undesirable setting risk,
because some GUCs have special meaning for 0.

And then I made a patch for this.
Please check the attached patch.

The patch also rounds a zero up to one. A naked zero with no unit is not
affected, but e.g if you have "log_rotation_age=0s", it will not disable
the feature as you might expect, but set it to 1 minute. Should we do
something about that?

If we're going to explain the rounding up in the manual, we also need to
explain the normal rule, which is to round down. How about this:

--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -44,6 +44,15 @@
       (seconds), <literal>min</literal> (minutes), <literal>h</literal>
       (hours), and <literal>d</literal> (days).  Note that the multiplier
       for memory units is 1024, not 1000.
+
+    <para>
+     If a memory or time setting is specified with more precision than the
+     implicit unit of the setting, it is rounded down.  However, if 
rounding
+     down would yield a zero, it is rounded up to one instead.  For 
example,
+     the implicit unit of <varname>log_rotation_age</varname is 
minutes, so if
+     you set it to <literal>150s</literal>, it will be rounded down to two
+     minutes. However, if you set it to <literal>10s</literal>, it will be
+     rounded up to one minute.
      </para>

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Heikki Linnakangas <hlinnakangas@vmware.com> writes:

The patch also rounds a zero up to one. A naked zero with no unit is not
affected, but e.g if you have "log_rotation_age=0s", it will not disable
the feature as you might expect, but set it to 1 minute. Should we do
something about that?

That sounds like a dealbreaker to me. There are enough places where zero
has special meaning that we should not *ever* change zero to non-zero
silently.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 8/21/14 11:16 AM, Tom Lane wrote:

Heikki Linnakangas <hlinnakangas@vmware.com> writes:

The patch also rounds a zero up to one. A naked zero with no unit is not
affected, but e.g if you have "log_rotation_age=0s", it will not disable
the feature as you might expect, but set it to 1 minute. Should we do
something about that?

That sounds like a dealbreaker to me. There are enough places where zero
has special meaning that we should not *ever* change zero to non-zero
silently.

I don't think I like this idea anyway. If something has units of an
hour and the user (perhaps misunderstanding the setting) sets it to one
second, then we shouldn't silently change that to one hour.

If there is a problem with rounding it to zero, then we should perhaps
raise an error. (And stop treating zero specially. It's a terrible idea.)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut-2 wrote

On 8/21/14 11:16 AM, Tom Lane wrote:

Heikki Linnakangas &lt;

hlinnakangas@

&gt; writes:

The patch also rounds a zero up to one. A naked zero with no unit is not
affected, but e.g if you have "log_rotation_age=0s", it will not disable
the feature as you might expect, but set it to 1 minute. Should we do
something about that?

That sounds like a dealbreaker to me. There are enough places where zero
has special meaning that we should not *ever* change zero to non-zero
silently.

I don't think I like this idea anyway. If something has units of an
hour and the user (perhaps misunderstanding the setting) sets it to one
second, then we shouldn't silently change that to one hour.

If there is a problem with rounding it to zero, then we should perhaps
raise an error. (And stop treating zero specially. It's a terrible
idea.)

I'm on board, from the original thread, for errors if the input cannot be
converted to the parameter measurement unit cleanly. By which I mean the
specified value should result in an integer being recorded without rounding.
Specifying a precision less than the default unit thus becomes impossible.

I don't have a problem with zero meaning disabled when appropriate since it
avoids having a separate on/off GUC.

That said the complaint here just seems like a bug in the supplied patch -
zero is zero regardless of whether a unit is specified. The only obvious
exception would be temperature but that isn't relevant here.

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5815770.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Thank you for the comments.

It was a bug in my patch as another developer says.
I've not considered about the value 'zero', sorry.

I attached new patch.
This patch rounds up the value when only it's less than required unit.
Like below.
(unit: min)
0->0
0s->0
10s->1
70s->1

Although my original complaint is fixed, I'm worried about this change will
make users confusing.
Is it better to raise a message(ex. INFO) when a value less than required
unit is set?

2014-08-21 21:00 GMT+09:00 Heikki Linnakangas <hlinnakangas@vmware.com>:

Tomonari Katsumata <t.katsumata1122@gmail.com> writes:

This patch rounds up the value when only it's less than required unit.
..
Although my original complaint is fixed, I'm worried about this change will
make users confusing.

Indeed. I have not understood why you are insisting on "round up"
semantics. Wouldn't it make more sense for the behavior to be "round to
nearest"? That would get rid of any worries about treating zero specially.

Is it better to raise a message(ex. INFO) when a value less than required
unit is set?

No. Non-error messages are probably completely useless in this area:
users will typically never see such messages for settings made in
postgresql.conf, because it will not occur to them to look in the
postmaster log. So the behavior needs to be self-explanatory without
any messages; and that means it had better not be very complicated.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane-2 wrote

Tomonari Katsumata &lt;

t.katsumata1122@

&gt; writes:

This patch rounds up the value when only it's less than required unit.
..
Although my original complaint is fixed, I'm worried about this change
will
make users confusing.

Indeed. I have not understood why you are insisting on "round up"
semantics. Wouldn't it make more sense for the behavior to be "round to
nearest"? That would get rid of any worries about treating zero
specially.

Wasn't the goal that all non-zero values result in the feature being
enabled? With round nearest there will still be some values that are
non-zero but that round to zero and thus disable the feature.

Values failing in the range (0, 1) in the canonical unit must be treated
specially otherwise we might as well just leave the current behavior as-is
since floor is likely just as good a rule as round-nearest.

For fractions greater than one round nearest is probably fine and indeed on
average results in the least amount of potential adjustment magnitude.

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816007.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

David G Johnston <david.g.johnston@gmail.com> writes:

Tom Lane-2 wrote

Indeed. I have not understood why you are insisting on "round up"
semantics. Wouldn't it make more sense for the behavior to be "round to
nearest"? That would get rid of any worries about treating zero
specially.

Wasn't the goal that all non-zero values result in the feature being
enabled? With round nearest there will still be some values that are
non-zero but that round to zero and thus disable the feature.

Ah. Okay, but then what's wrong with the original proposal of "use ceil()
instead of floor()"? Basically I think the idea of treating fractions
less than one differently from fractions greater than one is bogus; nobody
will ever find that intuitive.

Or we could adopt Peter's idea that zero shouldn't be special (instead
using, say, -1 to turn things off). But I'm afraid that would break way
too many peoples' configuration choices.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Ah. Okay, but then what's wrong with the original proposal of "use ceil()
instead of floor()"? Basically I think the idea of treating fractions
less than one differently from fractions greater than one is bogus; nobody
will ever find that intuitive.

Or make it an error to specify a value that rounds to 0 but isn't 0.

--
greg

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane-2 wrote

David G Johnston &lt;

david.g.johnston@

&gt; writes:

Tom Lane-2 wrote

Indeed. I have not understood why you are insisting on "round up"
semantics. Wouldn't it make more sense for the behavior to be "round to
nearest"? That would get rid of any worries about treating zero
specially.

Wasn't the goal that all non-zero values result in the feature being
enabled? With round nearest there will still be some values that are
non-zero but that round to zero and thus disable the feature.

Ah. Okay, but then what's wrong with the original proposal of "use ceil()
instead of floor()"? Basically I think the idea of treating fractions
less than one differently from fractions greater than one is bogus; nobody
will ever find that intuitive.

Or we could adopt Peter's idea that zero shouldn't be special (instead
using, say, -1 to turn things off). But I'm afraid that would break way
too many peoples' configuration choices.

Yes, using ceil() is the most acceptable, for me, solution that does not
involve throwing an error.

Are there any examples of where treating zero specially is a problem or is
this concern theoretical? We've already decided to risk enabling disabled
features by applying this patch since the likelihood of someone relying on
the rounding to keep the feature disabled (or at its default value in some
instances) is unlikely.

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816018.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Aug 23, 2014 at 6:39 PM, Greg Stark <stark@mit.edu> wrote:

On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Ah. Okay, but then what's wrong with the original proposal of "use ceil()
instead of floor()"? Basically I think the idea of treating fractions
less than one differently from fractions greater than one is bogus; nobody
will ever find that intuitive.

Or make it an error to specify a value that rounds to 0 but isn't 0.

I liked David Johnston's even stronger suggestion upthread: make it an
error to specify a value requires rounding of any kind. In other
words, if the minimum granularity is 1 minute, you can specify that as
60 seconds instead, but if you write 59 seconds, we error out. Maybe
that seems pedantic, but I don't think users will much appreciate the
discovery that 30 seconds means 60 seconds. They'll be happier to be
told that up front than having to work it out afterward.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-08-26 16:16:32 -0400, Robert Haas wrote:

On Sat, Aug 23, 2014 at 6:39 PM, Greg Stark <stark@mit.edu> wrote:

On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Ah. Okay, but then what's wrong with the original proposal of "use ceil()
instead of floor()"? Basically I think the idea of treating fractions
less than one differently from fractions greater than one is bogus; nobody
will ever find that intuitive.

Or make it an error to specify a value that rounds to 0 but isn't 0.

I liked David Johnston's even stronger suggestion upthread: make it an
error to specify a value requires rounding of any kind. In other
words, if the minimum granularity is 1 minute, you can specify that as
60 seconds instead, but if you write 59 seconds, we error out. Maybe
that seems pedantic, but I don't think users will much appreciate the
discovery that 30 seconds means 60 seconds. They'll be happier to be
told that up front than having to work it out afterward.

Is the whole topic actually practically relevant? Afaics there's no
guc's with a time unit bigger than GUC_UNIT_S except log_rotation_age
where it surely doesn't matter and no space unit bigger than
GUC_UNIT_BLOCKS/GUC_UNIT_XBLOCKS.
In theory I agree with you/$subject, but I don't really see this worth
much effort.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 8/23/14 6:39 PM, Greg Stark wrote:

Or make it an error to specify a value that rounds to 0 but isn't 0.

That's what I would prefer.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 8/26/14 4:22 PM, Andres Freund wrote:

Is the whole topic actually practically relevant?

It's clearly not all that important, or otherwise we'd have heard about
before now.

I suppose someone could do something like

wal_receiver_status_interval = 10ms

and end up silently turning the whole thing off instead of making it
very aggressive.

The mistake here is that the mathematically appropriate turn-off value
in this and similar cases is infinity, not zero.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

I liked David Johnston's even stronger suggestion upthread: make it an
error to specify a value requires rounding of any kind. In other
words, if the minimum granularity is 1 minute, you can specify that as
60 seconds instead, but if you write 59 seconds, we error out. Maybe
that seems pedantic, but I don't think users will much appreciate the
discovery that 30 seconds means 60 seconds. They'll be happier to be
told that up front than having to work it out afterward.

I think this is totally wrong. The entire point of the GUC units system,
or at least a large part of the point, is that users should not have to
be intimately aware of the units in which a given value is measured
internally. And that in turn means that the units had better be such
that users won't find them overly coarse. If it matters a lot whether
59 seconds gets rounded to 60, then we didn't make a good choice of units
for the GUC in question; and we should fix that choice, not mess with the
rounding rules.

The case where this argument falls down is for "special" values, such as
where zero means something quite different from the smallest nonzero
value. Peter suggested upthread that we should redefine any GUC values
for which that is true, but (a) I think that loses on backwards
compatibility grounds, and (b) ISTM zero is probably always special to
some extent. A zero time delay for example is not likely to work.

Maybe we should leave the rounding behavior alone (there's not much
evidence that rounding in one direction is worse than another; although
I'd also be okay with changing to round-to-nearest), and confine ourselves
to throwing an error for the single case that an apparently nonzero input
value is truncated/rounded to zero as a result of units conversion.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane-2 wrote

Robert Haas &lt;

robertmhaas@

&gt; writes:

I liked David Johnston's even stronger suggestion upthread: make it an
error to specify a value requires rounding of any kind. In other
words, if the minimum granularity is 1 minute, you can specify that as
60 seconds instead, but if you write 59 seconds, we error out. Maybe
that seems pedantic, but I don't think users will much appreciate the
discovery that 30 seconds means 60 seconds. They'll be happier to be
told that up front than having to work it out afterward.

I think this is totally wrong. The entire point of the GUC units system,
or at least a large part of the point, is that users should not have to
be intimately aware of the units in which a given value is measured
internally. And that in turn means that the units had better be such
that users won't find them overly coarse. If it matters a lot whether
59 seconds gets rounded to 60, then we didn't make a good choice of units
for the GUC in question; and we should fix that choice, not mess with the
rounding rules.

The case where this argument falls down is for "special" values, such as
where zero means something quite different from the smallest nonzero
value. Peter suggested upthread that we should redefine any GUC values
for which that is true, but (a) I think that loses on backwards
compatibility grounds, and (b) ISTM zero is probably always special to
some extent. A zero time delay for example is not likely to work.

Maybe we should leave the rounding behavior alone (there's not much
evidence that rounding in one direction is worse than another; although
I'd also be okay with changing to round-to-nearest), and confine ourselves
to throwing an error for the single case that an apparently nonzero input
value is truncated/rounded to zero as a result of units conversion.

To Andres' point:

SELECT unit, count(*) FROM pg_settings WHERE unit <> '' GROUP BY unit; (9.3
/ Ubuntu)

min (1 - log_rotation_age)
s (10)
ms (13)

kb (7)
8kb (6)

I don't know about the size implications but they seem to be non-existent.
That any setting critically matters at +/- 1s or 1ms doesn't seem likely in
practice. Even +/- 1min for a setting, if it did matter at extreme scale,
would be recognizable by the user in practice as a rounding artifact and
compensated for.

At this point throwing an error for any precision that results in less than
the default precision is my preference. I would not change the rounding
rules for the simple reason that there is no obvious improvement to be had
and so why introduce pointless change that - however marginal and unlikely -
will be user-visible.

The complaint to overcome is avoiding an interpretation of "zero" when the
precision of the input is less than the GUC unit. Lacking any concrete
complaints about our round-down policy I don't see where a change there is
worthwhile.

Fixing zero as a special value falls under the same category. As
mathematically pure as using infinity may be the trade-off for practicality
and usability seems, even in light of this complaint, like the correct one
to have made.

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816409.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

I'm sorry for slow reaction.

I don't care whether rounding up or down it, although this title has
'rounding up'.
(I just only come up with it. I'm sorry for my imprudence)

I'm thinking about a method which users get quick awareness it.
Now, it's okay not to change current behavior except non-zero value yields
a zero. A zero rounded down from non-zero gets an error.

I attached new patch.
This includes a document about above behavior as Heikki suggested.

regards,
--------------
Tomonari Katsumata

2014-08-27 6:49 GMT+09:00 David G Johnston <david.g.johnston@gmail.com>: