Skip site navigation (1) Skip section navigation (2)

Re: [HACKERS] Interval aggregate regression failure (expected seems

From: Michael Glaesemann <grzm(at)seespotcode(dot)net>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paesold <mpaesold(at)gmx(dot)at>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [HACKERS] Interval aggregate regression failure (expected seems
Date: 2006-08-30 01:30:43
Message-ID: BC39C94F-97B5-4FEF-AAD7-0069EE4F3688@seespotcode.net (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
On Aug 30, 2006, at 7:12 , Bruce Momjian wrote:

> Here are the results using my newest patch:
>
> 	test=> select interval '41 mon 12 days 360:00' / 10 as quotient_a
> 	     , interval '41 mon -12 days -360:00' / 10 as quotient_b
> 	     , interval '-41 mon 12 days 360:00' / 10 as quotient_c
> 	     , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
> 	       quotient_a       |       quotient_b        |         
> quotient_c         |        quotient_d
> 	------------------------+------------------------- 
> +---------------------------+---------------------------
> 	 4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2  
> days +40:48:00 | -4 mons -4 days -40:48:00
> 	(1 row)
> 	
> 	test=> select interval '41 mon 12 days 360:00' * 0.3 as product_a
> 	     , interval '41 mon -12 days -360:00' * 0.3 as product_b
> 	     , interval '-41 mon 12 days 360:00' * 0.3 as product_c
> 	     , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
> 	        product_a         |        product_b         |           
> product_c          |          product_d
> 	--------------------------+-------------------------- 
> +-----------------------------+------------------------------
> 	 1 year 12 days 122:24:00 | 1 year 6 days -122:24:00 | -1 years -6  
> days +122:24:00 | -1 years -12 days -122:24:00
> 	(1 row)
>
> I see no "23:60" entries.

Using Bruce's newest patch, I still get the "23:60" entries on my  
machine (no integer-datetimes)

select version();
                                                                       
version
------------------------------------------------------------------------ 
------------------------------------------------------------------------ 
-
PostgreSQL 8.2devel on powerpc-apple-darwin8.7.0, compiled by GCC  
powerpc-apple-darwin8-gcc-4.0.1 (GCC) 4.0.1 (Apple Computer, Inc.  
build 5341)
(1 row)

select interval '41 mon 12 days 360:00' / 10 as quotient_a
     , interval '41 mon -12 days -360:00' / 10 as quotient_b
     , interval '-41 mon 12 days 360:00' / 10 as quotient_c
     , interval '-41 mon -12 days -360:00' / 10 as quotient_d;
        quotient_a       |       quotient_b        |         
quotient_c         |        quotient_d
------------------------+------------------------- 
+---------------------------+---------------------------
4 mons 4 days 40:48:00 | 4 mons 2 days -40:48:00 | -4 mons -2 days  
+40:48:00 | -4 mons -4 days -40:48:00
(1 row)

select interval '41 mon 12 days 360:00' * 0.3 as product_a
     , interval '41 mon -12 days -360:00' * 0.3 as product_b
     , interval '-41 mon 12 days 360:00' * 0.3 as product_c
     , interval '-41 mon -12 days -360:00' * 0.3 as product_d;
         product_a         |          product_b          |           
product_c          |            product_d
--------------------------+----------------------------- 
+-----------------------------+---------------------------------
1 year 12 days 122:24:00 | 1 year 6 days -122:23:60.00 | -1 years -6  
days +122:24:00 | -1 years -12 days -122:23:60.00
(1 row)


> The code assume if it is within 0.000001 of a whole number, it  
> should be
> rounded to a whole number. Patch attached with comments added.

>   	/* fractional months full days into days */
>   	month_remainder_days = month_remainder * DAYS_PER_MONTH;
> + 	/*
> + 	 *	The remainders suffer from float rounding, so if they are
> + 	 *	within 0.000001 of an integer, we round them to integers.
> + 	 */
> + 	if (month_remainder_days != (int32)month_remainder_days &&
> + 		TSROUND(month_remainder_days) == rint(month_remainder_days))
> + 		month_remainder_days = rint(month_remainder_days);
>   	result->day += (int32) month_remainder_days;
>

Don't we want to be checking for rounding at the usec level rather  
than 0.000001 of a day? I think this should be

	if (month_remainder_days != (int32)month_remainder_days &&
		TSROUND(month_remainder_days * SECS_PER_DAY) ==
									rint(month_remainder_days * SECS_PER_DAY))
		month_remainder_days = rint(month_remainder_days);

Another question I have concerns the month_remainder_days != (int32)  
month_remainder_days comparison. If I understand it correctly, if the  
TSROUND == rint portion is true, the first part is true. Or is this  
just a quick, fast check to see if it's necessary to do a more  
computationally intensive check?

TSROUND isn't defined for HAVE_INT64_TIMESTAMP. My first attempt at  
performing a corresponding comparison doesn't work:

+ 	if (month_remainder_days != (int32) month_remainder_days &&
+ #ifdef HAVE_INT64_TIMESTAMP
+ 		rint(month_remainder_days * USECS_PER_DAY) ==
+ 		   (month_remainder_days * USECS_PER_DAY))
+ #else
+ 		TSROUND(month_remainder_days * SECS_PER_DAY) ==
+ 		   rint(month_remainder_days * SECS_PER_DAY))
+ #endif
+ 		month_remainder_days = rint(month_remainder_days);

Anyway, I'll pound on this some more tonight.

Michael Glaesemann
grzm seespotcode net

----------------------------------------

Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.165
diff -c -r1.165 timestamp.c
*** src/backend/utils/adt/timestamp.c	13 Jul 2006 16:49:16 -0000	1.165
--- src/backend/utils/adt/timestamp.c	30 Aug 2006 00:48:37 -0000
***************
*** 2518,2523 ****
--- 2518,2536 ----

   	/* fractional months full days into days */
   	month_remainder_days = month_remainder * DAYS_PER_MONTH;
+ 	/*
+ 	 *	The remainders suffer from float rounding, so if they are
+ 	 *	within 0.000001 of an integer, we round them to integers.
+ 	 */
+ 	if (month_remainder_days != (int32) month_remainder_days &&
+ #ifdef HAVE_INT64_TIMESTAMP
+ 		rint(month_remainder_days * USECS_PER_DAY) ==
+ 		   (month_remainder_days * USECS_PER_DAY))
+ #else
+ 		TSROUND(month_remainder_days * SECS_PER_DAY) ==
+ 		   rint(month_remainder_days * SECS_PER_DAY))
+ #endif
+ 		month_remainder_days = rint(month_remainder_days);
   	result->day += (int32) month_remainder_days;
   	/* fractional months partial days into time */
   	day_remainder += month_remainder_days - (int32)  
month_remainder_days;
***************
*** 2571,2576 ****
--- 2584,2602 ----

   	/* fractional months full days into days */
   	month_remainder_days = month_remainder * DAYS_PER_MONTH;
+ 	/*
+ 	 *	The remainders suffer from float rounding, so if they are
+ 	 *	within 0.000001 of an integer, we round them to integers.
+ 	 */
+ 	if (month_remainder_days != (int32) month_remainder_days &&
+ #ifdef HAVE_INT64_TIMESTAMP
+ 		rint(month_remainder_days * USECS_PER_DAY) ==
+ 		   (month_remainder_days * USECS_PER_DAY))
+ #else
+ 		TSROUND(month_remainder_days * SECS_PER_DAY) ==
+ 		   rint(month_remainder_days * SECS_PER_DAY))
+ #endif
+ 		month_remainder_days = rint(month_remainder_days);
   	result->day += (int32) month_remainder_days;
   	/* fractional months partial days into time */
   	day_remainder += month_remainder_days - (int32)  
month_remainder_days;


In response to

Responses

pgsql-hackers by date

Next:From: ITAGAKI TakahiroDate: 2006-08-30 02:46:49
Subject: Re: stats test on Windows is now failing repeatably?
Previous:From: Joshua D. DrakeDate: 2006-08-30 00:45:21
Subject: Re: Santa Clara Pg Training Event

pgsql-patches by date

Next:From: Bruce MomjianDate: 2006-08-30 03:50:40
Subject: Re: [HACKERS] Interval aggregate regression failure
Previous:From: Tom LaneDate: 2006-08-29 22:31:24
Subject: Re: updated patch for selecting large results sets in psql using cursors

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group