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:



---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iEYEARECAAYFAkJJW0EACgkQLQ/DKuwDYznWWgCgi+qDYzP9B2F69qjE19o+PK6Q
H2kAn3pRyELIP+7DjekaBJVsMqzCqtHZ
=AHA1
-----END PGP SIGNATURE-----

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);



---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match