Thread: Re: [BUGS] BUG #1563: wrong week returnded by date_trunc('week',
Tom Lane wrote: > Robert Creager <Robert_Creager@LogicalChaos.org> writes: > > "Dirk Raetzel" <d00273@spaetzle.de> 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:
When grilled further on (Tue, 29 Mar 2005 07:43:53 +0200), Dirk Raetzel <d00273@spaetzle.de> confessed: > > > On Mon, 28 Mar 2005, Bruce Momjian wrote: > > > 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: > > The problem arises as well when week number is 52 as in '2006-01-01'. Try these at the least: select date_trunc_week( '2004-01-01' ); -- should be 2003-12-29 00:00:00 select date_trunc_week( '2005-01-01' ); -- should be 2004-12-27 00:00:00 select date_trunc_week( '2005-06-01' ); -- should be 2005-05-30 00:00:00 select date_trunc_week( '2006-01-01' ); -- should be 2005-12-26 00:00:00 select date_trunc_week( '2007-01-01' ); -- should be 2007-01-01 00:00:00 If it helps, look earlier in this thread for a (overly complicated?) version in plpgsql. Cheers, Rob -- 06:39:48 up 1 day, 3 min, 9 users, load average: 0.10, 0.24, 0.30 Linux 2.6.5-02 #8 SMP Mon Jul 12 21:34:44 MDT 2004
Attachment
Robert Creager wrote: -- Start of PGP signed section. > When grilled further on (Tue, 29 Mar 2005 07:43:53 +0200), > Dirk Raetzel <d00273@spaetzle.de> confessed: > > > > > > > On Mon, 28 Mar 2005, Bruce Momjian wrote: > > > > > 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: > > > > The problem arises as well when week number is 52 as in '2006-01-01'. > > Try these at the least: > > select date_trunc_week( '2004-01-01' ); -- should be 2003-12-29 00:00:00 > select date_trunc_week( '2005-01-01' ); -- should be 2004-12-27 00:00:00 > select date_trunc_week( '2005-06-01' ); -- should be 2005-05-30 00:00:00 > select date_trunc_week( '2006-01-01' ); -- should be 2005-12-26 00:00:00 > select date_trunc_week( '2007-01-01' ); -- should be 2007-01-01 00:00:00 > > If it helps, look earlier in this thread for a (overly complicated?) version in > plpgsql. Wow, an early January date can be part of the 52nd week of the previous year too. Here is a new patch, with a documentation mention. -- 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: doc/src/sgml/func.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v retrieving revision 1.242 diff -c -c -r1.242 func.sgml *** doc/src/sgml/func.sgml 25 Mar 2005 16:08:39 -0000 1.242 --- doc/src/sgml/func.sgml 30 Mar 2005 06:04:39 -0000 *************** *** 5472,5477 **** --- 5472,5483 ---- week starts on Monday.) In other words, the first Thursday of a year is in week 1 of that year. (for <type>timestamp</type> values only) </para> + <para> + Because of this, it is possible for early January dates to be part of the + 52nd or 53rd week of the previous year. For example, <literal>2005-01-01</> + is part of the 53rd week of year 2004, and <literal>2006-01-01</> is part of + the 52nd week of year 2005. + </para> <screen> SELECT EXTRACT(WEEK FROM TIMESTAMP '2001-02-16 20:38:40'); 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 30 Mar 2005 06:04:41 -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 week 52/53 and the month is January, ! * then the week must belong to the previous year. ! */ ! if (woy >= 52 && 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 week 52/53 and the month is January, ! * then the week must belong to the previous year. ! */ ! if (woy >= 52 && 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: *************** *** 3142,3148 **** * Sometimes the last few days in a year will fall into the first week * of the next year, so check for this. */ ! if (result >= 53) { day4 = date2j(year + 1, 1, 4); --- 3164,3170 ---- * Sometimes the last few days in a year will fall into the first week * of the next year, so check for this. */ ! if (result >= 52) { day4 = date2j(year + 1, 1, 4); *************** *** 3198,3204 **** * Sometimes the last few days in a year will fall into the first week * of the next year, so check for this. */ ! if (result >= 53) { day4 = date2j(year + 1, 1, 4); --- 3220,3226 ---- * Sometimes the last few days in a year will fall into the first week * of the next year, so check for this. */ ! if (result >= 52) { day4 = date2j(year + 1, 1, 4);
On Mon, 28 Mar 2005, Bruce Momjian wrote: > 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: The problem arises as well when week number is 52 as in '2006-01-01'. Dirk