Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Ron Mayer

Kevin Grittner wrote:
Kevin Grittner [EMAIL PROTECTED] wrote: 
 
Even more surprising is the behavior for interval(1) here:

[ some context with nonsurprising examples removed ...]
ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(1);
 interval
--
 1 year 2 mons 3 days 04:05:06.60
(1 row)

That trailing zero should be considered a bug.


Is there a consensus that we don't want that trailing zero?
I notice that datetime.c's TrimTrailingZeros(char *str) has
the comment:
/* chop off trailing zeros... but leave at least 2 fractional digits */
that suggests that the trailing zero was intentional, but I
can't find any reasons why 2 fractional disgits were left.

The same function's also used for timestamps, so if we remove that
trailing zero in both places we'll see some regression differences
where we get
! | Mon Feb 10 17:32:01.5 1997 PST |1997 |7 |   1
instead of
! | Mon Feb 10 17:32:01.50 1997 PST |1997 |7 |   1

IMHO we don't want the extra zero for timestamps either.


If people agree I'll fold it into the patch dealing with
the other interval rounding eccentricities I have.

Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

[some other interval rounding example]


I don't much like the forced rounding to two digits here, but changing
that doesn't seem like material for back-patching.  Are you going to
fix that up while working on your other patches?





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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Kenneth Marshall
On Thu, Oct 09, 2008 at 11:50:17AM -0700, Ron Mayer wrote:
 Kevin Grittner wrote:
 Kevin Grittner [EMAIL PROTECTED] wrote: 
  Even more surprising is the behavior for interval(1) here:
 [ some context with nonsurprising examples removed ...]
 ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(1);
  interval
 --
  1 year 2 mons 3 days 04:05:06.60
 (1 row)
 That trailing zero should be considered a bug.

 Is there a consensus that we don't want that trailing zero?
 I notice that datetime.c's TrimTrailingZeros(char *str) has
 the comment:
 /* chop off trailing zeros... but leave at least 2 fractional digits */
 that suggests that the trailing zero was intentional, but I
 can't find any reasons why 2 fractional disgits were left.

 The same function's also used for timestamps, so if we remove that
 trailing zero in both places we'll see some regression differences
 where we get
 ! | Mon Feb 10 17:32:01.5 1997 PST |1997 |7 |   1
 instead of
 ! | Mon Feb 10 17:32:01.50 1997 PST |1997 |7 |   1

 IMHO we don't want the extra zero for timestamps either.

There is a difference between the result 0.6 and 0.60 in rounding.
The first is accurate +-0.05 and the second is +-0.005. Certainly,
it does not seem unreasonable that machines can calulate intervals
to the nearest 100th of a second. What is not clear to me is how the
decision to stop at the 2nd decimal digit was reached. If timestamps
are accurate to 1/100th, intervals should be returned to that level
of accuracy as well. Trailing digits definitely have meaning.

My 2 cents,
Ken


 If people agree I'll fold it into the patch dealing with
 the other interval rounding eccentricities I have.

 Tom Lane wrote:
 Ron Mayer [EMAIL PROTECTED] writes:
 [some other interval rounding example]
 I don't much like the forced rounding to two digits here, but changing
 that doesn't seem like material for back-patching.  Are you going to
 fix that up while working on your other patches?




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


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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Tom Lane
Kenneth Marshall [EMAIL PROTECTED] writes:
 There is a difference between the result 0.6 and 0.60 in rounding.
 The first is accurate +-0.05 and the second is +-0.005. Certainly,
 it does not seem unreasonable that machines can calulate intervals
 to the nearest 100th of a second. What is not clear to me is how the
 decision to stop at the 2nd decimal digit was reached.

Probably by flipping a coin ;-).  You have to remember that all this
behavior was designed around floating-point intervals, so there's
inherent imprecision in there; and the extent depends on the size of
the interval which makes it pretty hard to choose a display precision.

In the integer-timestamp world we know that the number is exact in
microseconds.  We clearly ought to be prepared to display up to six
fractional digits, but suppressing trailing zeroes in that seems
appropriate.

We could try to do the same in the float case, but I'm a bit worried
about finding ourselves showing 1234567.79 where it should be
1234567.8.

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Kenneth Marshall
On Thu, Oct 09, 2008 at 02:47:24PM -0500, Kevin Grittner wrote:
  Kenneth Marshall [EMAIL PROTECTED] wrote:
  
   Even more surprising is the behavior for interval(1) here:
  [ some context with nonsurprising examples removed ...]
  ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(1);
   interval
  --
   1 year 2 mons 3 days 04:05:06.60
  (1 row)
  That trailing zero should be considered a bug.
  
  What is not clear to me is how the
  decision to stop at the 2nd decimal digit was reached.
  
 See this posting and others on the thread:
  
 http://archives.postgresql.org/pgsql-hackers/2008-09/msg00999.php
  
 The current rules seem to be:
  
 (1)  If precision is specified, round to that precision.
  
 (2)  If result has only zeros in the fraction, show no fraction, else
 show at least two digits in the fraction, adding a trailing zero if
 needed to get to two digits, but don't show any trailing zeros in the
 fraction beyond the second position.
  
 I think it would be ideal if we could track how many digits of
 accuracy we have in a value, and show them all, even if that involves
 trailing zeros.  If that's not feasible, let's consistently not show
 trailing zeros.  Rounding .64 to .6 and then showing .60 is just plain
 wrong.
  
 -Kevin
 
+1

Ken

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Ron Mayer

Tom Lane wrote:

In the integer-timestamp world we know that the number is exact in
microseconds.  We clearly ought to be prepared to display up to six
fractional digits, but suppressing trailing zeroes in that seems
appropriate.


Great.


We could try to do the same in the float case, but I'm a bit worried
about finding ourselves showing 1234567.79 where it should be
1234567.8.


If I understand the code right fsec should mostly be values
between -1 and 1 anyway, because even in the floating point
case seconds are carried in the tm-tm_sec part.
It looks to me that a double should be plenty to do
microseconds so long as we don't put big numbers into fsec.
  printf(%.99f\n,59.11);
  59.11426907882 ...

Anyway - I'll try showing up-to-6-digits in both cases and
seeing if I can find a test case that breaks.

I guess this rounding trickiness with rounding explains some of
the bizarre code like this too:
   #if 0
/* chop off trailing one to cope with interval rounding */
if (strcmp(str + len - 4, 0001) == 0)
{
len -= 4;
*(str + len) = '\0';
}
   #endif

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Tom Lane
Ron Mayer [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 We could try to do the same in the float case, but I'm a bit worried
 about finding ourselves showing 1234567.79 where it should be
 1234567.8.

 If I understand the code right fsec should mostly be values
 between -1 and 1 anyway, because even in the floating point
 case seconds are carried in the tm-tm_sec part.

The problem is that that's a decomposed representation.  In the stored
form, there's a floating-point seconds field that includes hours,
minutes, seconds, and fractional seconds, and therefore large values
of the H/M/S fields degrade the accuracy of the fraction part.

Here's an example (testing in 8.3, since HEAD defaults to integer):

regression=# select '1234567890 hours 0.123 sec'::interval;
interval 
-
 1234567890:00:00.123047
(1 row)

Since there's a (somewhat arbitrary) limitation of the hours to 2^31,
this is close to the worst possible case.  (Hm, maybe someone actually
did the math and decided that 2 fractional digits were the most they
could promise given that?  No, because this code dates from a time
when we included days in the same field too ... back then there might
have been no accuracy at all in the fraction part.)

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Kevin Grittner
 Kenneth Marshall [EMAIL PROTECTED] wrote:
 
  Even more surprising is the behavior for interval(1) here:
 [ some context with nonsurprising examples removed ...]
 ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(1);
  interval
 --
  1 year 2 mons 3 days 04:05:06.60
 (1 row)
 That trailing zero should be considered a bug.
 
 What is not clear to me is how the
 decision to stop at the 2nd decimal digit was reached.
 
See this posting and others on the thread:
 
http://archives.postgresql.org/pgsql-hackers/2008-09/msg00999.php
 
The current rules seem to be:
 
(1)  If precision is specified, round to that precision.
 
(2)  If result has only zeros in the fraction, show no fraction, else
show at least two digits in the fraction, adding a trailing zero if
needed to get to two digits, but don't show any trailing zeros in the
fraction beyond the second position.
 
I think it would be ideal if we could track how many digits of
accuracy we have in a value, and show them all, even if that involves
trailing zeros.  If that's not feasible, let's consistently not show
trailing zeros.  Rounding .64 to .6 and then showing .60 is just plain
wrong.
 
-Kevin

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-10-09 Thread Ron Mayer

Tom Lane wrote:

Ron Mayer [EMAIL PROTECTED] writes:

Tom Lane wrote:

We could try to do the same in the float case, but I'm a bit worried
about finding ourselves showing 1234567.79 ...

If I understand the code right [I didn't...]


The problem is ... seconds field that includes hours,
minutes, seconds, and fractional seconds...Here's an example...
regression=# select '1234567890 hours 0.123 sec'::interval;
... 1234567890:00:00.123047


Hmm.  That's also an existence proof that we're not too concerned
about showing 6 imprecise digits anyway (at least for some 8.3
DateStyles).  Doesn't seem like it'd hurt too much if we show
them for all the IntervalStyles.


Since there's a (somewhat arbitrary) limitation of the hours to 2^31,
this is close to the worst possible case.  (Hm, maybe someone actually
did the math and decided that 2 fractional digits ...


Or I guess we could truncate to 2 digits only in the float case;
or truncate to 2 digits only if we're using the float case and
have large values.   But that extra complexity doesn't seem
worth it to me - especially since it seems to only affect
people who do two non-default things (pick a date/interval style
that used to truncate to 2, and --disable-integer-datetimes).

I put a patch up at http://0ape.com/postgres_interval_patches
that does what I think seems getting reasonable.   For better
or worse, it depends on the other two interval patches I was
working on, but I could make a version that doesn't depend on
those as well if people prefer that.

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-09-15 Thread Tom Lane
Ron Mayer [EMAIL PROTECTED] writes:
 Unless I'm compiling stuff wrong, it seems HEAD is giving me
 slightly different output on Intervals than 8.3 in the roundoff
 of seconds.   8.3 was rounding to the nearest fraction of a second,
 HEAD seems to be truncating.

Yeah, that's surely because of the change to integer timestamps.

 Am I interpreting this right?  If so, shall I submit a patch
 that rounds it to hundredths of a second (hundredths seems
 hardcoded in the sprintf), or perhaps just silently add that
 fix to the EncodeInterval patch I'm doing any for SQL Standard
 and ISO intervals?

This is not the only place where the float-timestamps code has rounding
behavior that doesn't appear in the integer-timestamps code.  I don't
know if we want to buy into making them act the same ... after all,
if they acted exactly the same, we'd not have bothered with writing the
integer code.  In this example, rounding to hundredths doesn't seem like
a particularly good idea; it seems to me it should give you the exact
down-to-the-microsecond value.

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-09-15 Thread Kevin Grittner
 On Mon, Sep 15, 2008 at  4:58 PM, in message
[EMAIL PROTECTED],
Tom Lane [EMAIL PROTECTED] wrote: 
 Ron Mayer [EMAIL PROTECTED] writes:
 Unless I'm compiling stuff wrong, it seems HEAD is giving me
 slightly different output on Intervals than 8.3 in the roundoff
 of seconds.   8.3 was rounding to the nearest fraction of a second,
 HEAD seems to be truncating.
 
 Yeah, that's surely because of the change to integer timestamps.
 
 Am I interpreting this right?  If so, shall I submit a patch
 that rounds it to hundredths of a second (hundredths seems
 hardcoded in the sprintf), or perhaps just silently add that
 fix to the EncodeInterval patch I'm doing any for SQL Standard
 and ISO intervals?
 
 This is not the only place where the float-timestamps code has
rounding
 behavior that doesn't appear in the integer-timestamps code.  I
don't
 know if we want to buy into making them act the same ... after all,
 if they acted exactly the same, we'd not have bothered with writing
the
 integer code.  In this example, rounding to hundredths doesn't seem
like
 a particularly good idea; it seems to me it should give you the
exact
 down-to-the-microsecond value.
 
I find the results on 8.3.3 with integer timestamps surprising:
 
ccdev=# select version();
 version
--
 PostgreSQL 8.3.3 on x86_64-unknown-linux-gnu, compiled by GCC gcc
(GCC) 4.1.2 20070115 (prerelease) (SUSE Linux)
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.69'::interval;
   interval
--
 1 year 2 mons 3 days 04:05:06.69
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.69'::interval(1);
 interval
--
 1 year 2 mons 3 days 04:05:06.70
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.69'::interval(2);
 interval
--
 1 year 2 mons 3 days 04:05:06.70
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.69'::interval(3);
 interval
--
 1 year 2 mons 3 days 04:05:06.70
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.69'::interval(4);
 interval
--
 1 year 2 mons 3 days 04:05:06.70
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.69'::interval(5);
 interval
--
 1 year 2 mons 3 days 04:05:06.70
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.69'::interval(6);
   interval
--
 1 year 2 mons 3 days 04:05:06.69
(1 row)
 
-Kevin

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-09-15 Thread Kevin Grittner
 Kevin Grittner [EMAIL PROTECTED] wrote: 
 
 I find the results on 8.3.3 with integer timestamps surprising:
 
Even more surprising is the behavior for interval(1) here:
 
ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval;
  interval
-
 1 year 2 mons 3 days 04:05:06.64321
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(1);
 interval
--
 1 year 2 mons 3 days 04:05:06.60
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(2);
 interval
--
 1 year 2 mons 3 days 04:05:06.64
(1 row)

ccdev=# select '1 year 2 mons 3 days 04:05:06.64321'::interval(3);
 interval
---
 1 year 2 mons 3 days 04:05:06.643
(1 row)
 
etc.
 
That trailing zero should be considered a bug.
 
-Kevin

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-09-15 Thread Ron Mayer

Kevin Grittner wrote:

...not the only place where the float-timestamps code has
rounding behavior that doesn't appear in the integer-timestamps 
code. ...  
I find the results on 8.3.3 with integer timestamps surprising:


Agreed it's surprising and agreed there are more places.

Sounds like I should keep that separate and perhaps later
submit a separate patch to identify and/or remove surprising
rounding behavior.

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-09-15 Thread Tom Lane
Ron Mayer [EMAIL PROTECTED] writes:
 Sounds like I should keep that separate and perhaps later
 submit a separate patch to identify and/or remove surprising
 rounding behavior.

Agreed.  It's easier to review and get consensus on patches if you
keep separate issues separate.

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


Re: [HACKERS] 8.3 vs HEAD difference in Interval output?

2008-09-15 Thread Ron Mayer

Tom Lane wrote:


This is not the only place where the float-timestamps code has rounding
behavior that doesn't appear in the integer-timestamps code. 


Yeah... For that matter, I find this surprising as well:

regression=# select '0.7 secs'::interval, ('7 secs'::interval/10);
interval |  ?column?
-+-
 00:00:00.69 | 00:00:00.70
(1 row)

I'll start making a list of them for a future patch down the road.

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