Tom Lane wrote:
> Robert Creager <[EMAIL PROTECTED]> writes:
> > "Dirk Raetzel" <[EMAIL PROTECTED]> confessed:
> >> date_trunc('week', ...) returns the wrong week for first days in January if
> >> their calendar week belongs to the previous week.
> 
> > I brought this up a couple of weeks ago in Hackers since I created this 
> > error
> > last year :-(
> 
> I don't recall seeing that ... anyway, the problem seems to be that

I don't remember seeing it either.

> timestamp_trunc implements this as
> 
>         case DTK_WEEK:
>             isoweek2date(date2isoweek(tm->tm_year, tm->tm_mon, tm->tm_mday),
>                          &(tm->tm_year), &(tm->tm_mon), &(tm->tm_mday));
>             tm->tm_hour = 0;
>             tm->tm_min = 0;
>             tm->tm_sec = 0;
>             fsec = 0;
>             break;
> 
> which looks plausible on its face ... but given 2005-01-01, date2isoweek
> returns 53 --- which represents the 53rd week of 2004, which is correct
> --- and then isoweek2date thinks it is supposed to compute the 53rd week
> of 2005, which is not what's wanted.
> 
> We need to change the function APIs so that date2isoweek passes back
> some indication of which year it thought the week belongs to, and then
> isoweek2date must use that instead of the original year number.
> 
> Each of these functions is used in several places, so the change is not
> quite trivial, but still not a big deal.  Who wants to fix it?

I have developed a patch to fix the problem.  Instead of changing the
API, I added code to decrement the year when the week number was 53 and
the month was January.  It corrected the problem:
        
        test=> select date_trunc('week', timestamp '2005-01-01');
             date_trunc
        ---------------------
         2004-12-27 00:00:00
        (1 row)
        
        test=> select date_trunc('week', timestamptz '2005-01-01');
               date_trunc
        ------------------------
         2004-12-27 00:00:00-05
        (1 row)

        test=> select date_trunc('week', date '2005-01-01');
               date_trunc
        ------------------------
         2004-12-27 00:00:00-05
        (1 row)

It seems the idea of returning the week number and assuming the year is
the same is fundamentally flawed, but the user API is that way so I am
not inclined to adjust the server API at this point.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.117
diff -c -c -r1.117 timestamp.c
*** src/backend/utils/adt/timestamp.c   31 Dec 2004 22:01:22 -0000      1.117
--- src/backend/utils/adt/timestamp.c   28 Mar 2005 21:59:22 -0000
***************
*** 2754,2765 ****
                switch (val)
                {
                        case DTK_WEEK:
!                               isoweek2date(date2isoweek(tm->tm_year, 
tm->tm_mon, tm->tm_mday), &(tm->tm_year), &(tm->tm_mon), &(tm->tm_mday));
                                tm->tm_hour = 0;
                                tm->tm_min = 0;
                                tm->tm_sec = 0;
                                fsec = 0;
                                break;
                        case DTK_MILLENNIUM:
                                /* see comments in timestamptz_trunc */
                                if (tm->tm_year > 0)
--- 2754,2776 ----
                switch (val)
                {
                        case DTK_WEEK:
!                       {
!                               int woy;
!                               
!                               woy = date2isoweek(tm->tm_year, tm->tm_mon, 
tm->tm_mday);
!                               /*
!                                *      If it is the 53rd week and the month is 
January,
!                                *      then the week must belong to the 
previous year.
!                                */
!                               if (woy >= 53 && tm->tm_mon == 1)
!                                       --tm->tm_year;
!                               isoweek2date(woy, &(tm->tm_year), 
&(tm->tm_mon), &(tm->tm_mday));
                                tm->tm_hour = 0;
                                tm->tm_min = 0;
                                tm->tm_sec = 0;
                                fsec = 0;
                                break;
+                       }
                        case DTK_MILLENNIUM:
                                /* see comments in timestamptz_trunc */
                                if (tm->tm_year > 0)
***************
*** 2874,2886 ****
                switch (val)
                {
                        case DTK_WEEK:
!                               isoweek2date(date2isoweek(tm->tm_year, 
tm->tm_mon, tm->tm_mday), &(tm->tm_year), &(tm->tm_mon), &(tm->tm_mday));
                                tm->tm_hour = 0;
                                tm->tm_min = 0;
                                tm->tm_sec = 0;
                                fsec = 0;
                                redotz = true;
                                break;
                                /* one may consider DTK_THOUSAND and 
DTK_HUNDRED... */
                        case DTK_MILLENNIUM:
  
--- 2885,2908 ----
                switch (val)
                {
                        case DTK_WEEK:
!                       {
!                               int woy;
!                               
!                               woy = date2isoweek(tm->tm_year, tm->tm_mon, 
tm->tm_mday);
!                               /*
!                                *      If it is the 53rd week and the month is 
January,
!                                *      then the week must belong to the 
previous year.
!                                */
!                               if (woy >= 53 && tm->tm_mon == 1)
!                                       --tm->tm_year;
!                               isoweek2date(woy, &(tm->tm_year), 
&(tm->tm_mon), &(tm->tm_mday));
                                tm->tm_hour = 0;
                                tm->tm_min = 0;
                                tm->tm_sec = 0;
                                fsec = 0;
                                redotz = true;
                                break;
+                       }
                                /* one may consider DTK_THOUSAND and 
DTK_HUNDRED... */
                        case DTK_MILLENNIUM:
  
---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

Reply via email to