Re: [HACKERS] 8.3 vs HEAD difference in Interval output?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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