Thread: Re: [GENERAL] ISO week dates
Peter Eisentraut a ecrit le 07/10/2006 09:01:
> Brendan Jurd wrote:
>> * add an ISO day format pattern to to_char() called 'ID', which
>> starts at Monday = 1, and
>> * add an ISO year field to extract() called 'isoyear'?
>
> That seems reasonable. Do you volunteer?
>
I've tried to work on the first one, the ISO day field. My patch is
attached and is against CVS HEAD. It only takes care of the code,
nothing is done for documentation matter. It works with me :
toto=# select to_char(('2006-10-'||a+2)::date, 'DAY') as "dow",
to_char(('2006-10-'||a+2)::date, 'ID') as "ID field",
to_char(('2006-10-'||a+2)::date, 'D') as "D field"
from generate_series(1, 15) as a;
dow | ID field | D field
-----------+----------+---------
TUESDAY | 2 | 3
WEDNESDAY | 3 | 4
THURSDAY | 4 | 5
FRIDAY | 5 | 6
SATURDAY | 6 | 7
SUNDAY | 7 | 1
MONDAY | 1 | 2
TUESDAY | 2 | 3
WEDNESDAY | 3 | 4
THURSDAY | 4 | 5
FRIDAY | 5 | 6
SATURDAY | 6 | 7
SUNDAY | 7 | 1
MONDAY | 1 | 2
TUESDAY | 2 | 3
(15 rows)
I just want to know if my patch is interesting... and if it's OK, I can
work on the ISO year field.
Regards.
--
Guillaume.
Index: src/backend/utils/adt/formatting.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/formatting.c,v
retrieving revision 1.113
diff -r1.113 formatting.c
555a556
> DCH_ID,
599a601
> DCH_id,
698c700,701
< {"IW", 2, dch_date, DCH_IW, TRUE}, /* I */
---
> {"ID", 2, dch_date, DCH_ID, TRUE}, /* I */
> {"IW", 2, dch_date, DCH_IW, TRUE},
742c745,746
< {"iw", 2, dch_date, DCH_IW, TRUE}, /* i */
---
> {"id", 2, dch_date, DCH_ID, TRUE}, /* i */
> {"iw", 2, dch_date, DCH_IW, TRUE},
832c836
< DCH_FX, -1, DCH_HH24, DCH_IW, DCH_J, -1, -1, DCH_MI, -1, -1,
---
> DCH_FX, -1, DCH_HH24, DCH_ID, DCH_J, -1, -1, DCH_MI, -1, -1,
835c839
< DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_iw, DCH_j, -1, -1, DCH_mi,
---
> DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_id, DCH_j, -1, -1, DCH_mi,
2369a2374
> case DCH_ID:
2373c2378,2381
< sprintf(inout, "%d", tm->tm_wday + 1);
---
> if (arg == DCH_D)
> sprintf(inout, "%d", tm->tm_wday + 1);
> else
> sprintf(inout, "%d", (tm->tm_wday == 0) ? 7 : tm->tm_wday);
2380a2389,2390
> if (arg == DCH_ID && tmfc->d == 7)
> tmfc->d = 0;
Guillaume Lelarge wrote: > I've tried to work on the first one, the ISO day field. My patch is > attached and is against CVS HEAD. It only takes care of the code, > nothing is done for documentation matter. It works with me : I haven't been following this thread, but I just wanted to point out that we prefer context diffs. Please resend the patch as a context diff, using "diff -c" or "cvs diff -c", so that it's easier to review. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas a ecrit le 12/10/2006 12:43:
> Guillaume Lelarge wrote:
>> I've tried to work on the first one, the ISO day field. My patch is
>> attached and is against CVS HEAD. It only takes care of the code,
>> nothing is done for documentation matter. It works with me :
>
> I haven't been following this thread, but I just wanted to point out
> that we prefer context diffs.
>
> Please resend the patch as a context diff, using "diff -c" or "cvs diff
> -c", so that it's easier to review.
>
Sorry, you're right. Here it is.
--
Guillaume.
Index: src/backend/utils/adt/formatting.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/formatting.c,v
retrieving revision 1.113
diff -c -r1.113 formatting.c
*** src/backend/utils/adt/formatting.c 4 Oct 2006 00:29:59 -0000 1.113
--- src/backend/utils/adt/formatting.c 12 Oct 2006 10:54:01 -0000
***************
*** 553,558 ****
--- 553,559 ----
DCH_HH24,
DCH_HH12,
DCH_HH,
+ DCH_ID,
DCH_IW,
DCH_IYYY,
DCH_IYY,
***************
*** 597,602 ****
--- 598,604 ----
DCH_hh24,
DCH_hh12,
DCH_hh,
+ DCH_id,
DCH_iw,
DCH_iyyy,
DCH_iyy,
***************
*** 695,701 ****
{"HH24", 4, dch_time, DCH_HH24, TRUE}, /* H */
{"HH12", 4, dch_time, DCH_HH12, TRUE},
{"HH", 2, dch_time, DCH_HH, TRUE},
! {"IW", 2, dch_date, DCH_IW, TRUE}, /* I */
{"IYYY", 4, dch_date, DCH_IYYY, TRUE},
{"IYY", 3, dch_date, DCH_IYY, TRUE},
{"IY", 2, dch_date, DCH_IY, TRUE},
--- 697,704 ----
{"HH24", 4, dch_time, DCH_HH24, TRUE}, /* H */
{"HH12", 4, dch_time, DCH_HH12, TRUE},
{"HH", 2, dch_time, DCH_HH, TRUE},
! {"ID", 2, dch_date, DCH_ID, TRUE}, /* I */
! {"IW", 2, dch_date, DCH_IW, TRUE},
{"IYYY", 4, dch_date, DCH_IYYY, TRUE},
{"IYY", 3, dch_date, DCH_IYY, TRUE},
{"IY", 2, dch_date, DCH_IY, TRUE},
***************
*** 739,745 ****
{"hh24", 4, dch_time, DCH_HH24, TRUE}, /* h */
{"hh12", 4, dch_time, DCH_HH12, TRUE},
{"hh", 2, dch_time, DCH_HH, TRUE},
! {"iw", 2, dch_date, DCH_IW, TRUE}, /* i */
{"iyyy", 4, dch_date, DCH_IYYY, TRUE},
{"iyy", 3, dch_date, DCH_IYY, TRUE},
{"iy", 2, dch_date, DCH_IY, TRUE},
--- 742,749 ----
{"hh24", 4, dch_time, DCH_HH24, TRUE}, /* h */
{"hh12", 4, dch_time, DCH_HH12, TRUE},
{"hh", 2, dch_time, DCH_HH, TRUE},
! {"id", 2, dch_date, DCH_ID, TRUE}, /* i */
! {"iw", 2, dch_date, DCH_IW, TRUE},
{"iyyy", 4, dch_date, DCH_IYYY, TRUE},
{"iyy", 3, dch_date, DCH_IYY, TRUE},
{"iy", 2, dch_date, DCH_IY, TRUE},
***************
*** 829,838 ****
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
-1, -1, -1, -1, -1, DCH_A_D, DCH_B_C, DCH_CC, DCH_DAY, -1,
! DCH_FX, -1, DCH_HH24, DCH_IW, DCH_J, -1, -1, DCH_MI, -1, -1,
DCH_P_M, DCH_Q, DCH_RM, DCH_SSSS, DCH_TZ, DCH_US, -1, DCH_WW, -1, DCH_Y_YYY,
-1, -1, -1, -1, -1, -1, -1, DCH_a_d, DCH_b_c, DCH_cc,
! DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_iw, DCH_j, -1, -1, DCH_mi,
-1, -1, DCH_p_m, DCH_q, DCH_rm, DCH_ssss, DCH_tz, DCH_us, -1, DCH_ww,
-1, DCH_y_yyy, -1, -1, -1, -1
--- 833,842 ----
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
-1, -1, -1, -1, -1, DCH_A_D, DCH_B_C, DCH_CC, DCH_DAY, -1,
! DCH_FX, -1, DCH_HH24, DCH_ID, DCH_J, -1, -1, DCH_MI, -1, -1,
DCH_P_M, DCH_Q, DCH_RM, DCH_SSSS, DCH_TZ, DCH_US, -1, DCH_WW, -1, DCH_Y_YYY,
-1, -1, -1, -1, -1, -1, -1, DCH_a_d, DCH_b_c, DCH_cc,
! DCH_day, -1, DCH_fx, -1, DCH_hh24, DCH_id, DCH_j, -1, -1, DCH_mi,
-1, -1, DCH_p_m, DCH_q, DCH_rm, DCH_ssss, DCH_tz, DCH_us, -1, DCH_ww,
-1, DCH_y_yyy, -1, -1, -1, -1
***************
*** 2367,2376 ****
}
break;
case DCH_D:
INVALID_FOR_INTERVAL;
if (is_to_char)
{
! sprintf(inout, "%d", tm->tm_wday + 1);
if (S_THth(suf))
str_numth(p_inout, inout, S_TH_TYPE(suf));
return strlen(p_inout);
--- 2371,2384 ----
}
break;
case DCH_D:
+ case DCH_ID:
INVALID_FOR_INTERVAL;
if (is_to_char)
{
! if (arg == DCH_D)
! sprintf(inout, "%d", tm->tm_wday + 1);
! else
! sprintf(inout, "%d", (tm->tm_wday == 0) ? 7 : tm->tm_wday);
if (S_THth(suf))
str_numth(p_inout, inout, S_TH_TYPE(suf));
return strlen(p_inout);
***************
*** 2378,2383 ****
--- 2386,2393 ----
else
{
sscanf(inout, "%1d", &tmfc->d);
+ if (arg == DCH_ID && tmfc->d == 7)
+ tmfc->d = 0;
return strspace_len(inout) + 1 + SKIP_THth(suf);
}
break;
Guillaume Lelarge wrote:
> I've tried to work on the first one, the ISO day field. My patch is
> attached and is against CVS HEAD. It only takes care of the code,
> nothing is done for documentation matter. It works with me :
>
> toto=# select to_char(('2006-10-'||a+2)::date, 'DAY') as "dow",
> to_char(('2006-10-'||a+2)::date, 'ID') as "ID field",
> to_char(('2006-10-'||a+2)::date, 'D') as "D field"
> from generate_series(1, 15) as a;
There is an inconsistency here: 'IYYY' is the four-digit ISO year, 'IW'
is the two-digit ISO week, but 'ID' would be the one-digit ISO
day-of-the-week. I'm not sure we can fix that, but I wanted to point
it out.
We should also support a format for ISO day-of-the-year, which might
be 'IDDD'.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
Peter Eisentraut a écrit :
> Guillaume Lelarge wrote:
>> I've tried to work on the first one, the ISO day field. My patch is
>> attached and is against CVS HEAD. It only takes care of the code,
>> nothing is done for documentation matter. It works with me :
>>
>> toto=# select to_char(('2006-10-'||a+2)::date, 'DAY') as "dow",
>> to_char(('2006-10-'||a+2)::date, 'ID') as "ID field",
>> to_char(('2006-10-'||a+2)::date, 'D') as "D field"
>> from generate_series(1, 15) as a;
>
> There is an inconsistency here: 'IYYY' is the four-digit ISO year, 'IW'
> is the two-digit ISO week, but 'ID' would be the one-digit ISO
> day-of-the-week. I'm not sure we can fix that, but I wanted to point
> it out.
>
Is there a two digit ISO day of the week ? If not, we should use ID. As
you say, I don't know what we can do about that. I used Brendan Jurd's
idea, perhaps he can tell us more on this matter.
> We should also support a format for ISO day-of-the-year, which might
> be 'IDDD'.
>
I will work tomorrow on this one.
Regards.
--
Guillaume.
<!-- http://abs.traduc.org/
http://lfs.traduc.org/
http://traduc.postgresqlfr.org/ -->
Guillaume Lelarge a ecrit le 12/10/2006 20:20: > Peter Eisentraut a écrit : >> We should also support a format for ISO day-of-the-year, which might >> be 'IDDD'. >> > > I will work tomorrow on this one. > Don't we already have it ? It seems ISO day-of-the-year is between 001 and 366 in leap years. Isn't this the definition for DDD format ? if Im' right, I just need to add the IDDD pattern ? -- Guillaume.
Guillaume Lelarge wrote: > Guillaume Lelarge a ecrit le 12/10/2006 20:20: > > Peter Eisentraut a écrit : > >> We should also support a format for ISO day-of-the-year, which > >> might be 'IDDD'. > > > > I will work tomorrow on this one. > > Don't we already have it ? It seems ISO day-of-the-year is between > 001 and 366 in leap years. Isn't this the definition for DDD format ? > if Im' right, I just need to add the IDDD pattern ? The ISO 8601 day-of-the-year is aligned with the week-of-the-year. It should be the case that day one of week one is also day one of the year. (As a particular example, day one of 2006 is January 2, 2006.) -- Peter Eisentraut http://developer.postgresql.org/~petere/
On 10/13/06, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Peter Eisentraut a écrit : > > > > There is an inconsistency here: 'IYYY' is the four-digit ISO year, 'IW' > > is the two-digit ISO week, but 'ID' would be the one-digit ISO > > day-of-the-week. I'm not sure we can fix that, but I wanted to point > > it out. > > > > Is there a two digit ISO day of the week ? If not, we should use ID. As > you say, I don't know what we can do about that. I used Brendan Jurd's > idea, perhaps he can tell us more on this matter. > Thanks for your work so far Guillaume. I agree with Peter, it is inconsistent to have a one-digit field represented by a two-character code. However, I don't see a way around it. 'D' is already taken to mean the non-ISO day-of-week, and 'I' is taken to mean the last digit of the ISO year (although to be honest I don't see where this would be useful). This sort of thing is not unprecedented in to_char(). For example, the codes 'HH24' and 'HH12' are four characters long, but resolve to a two-digit result. 'DAY' resolves to nine characters, and so on. Basically I think we're stuck with ID for day-of-week and IDDD for day-of-year. I will take a look at implementing 'isoyear' for extract(), and also start putting together a patch for the documentation. If Guillaume is still interested in adding the IDDD field to to_char(), wonderful, if not I will pick up from his ID patch and add IDDD to it. Regards, BJ
Brendan Jurd a écrit : > On 10/13/06, Guillaume Lelarge <guillaume@lelarge.info> wrote: >> Peter Eisentraut a écrit : >> > >> > There is an inconsistency here: 'IYYY' is the four-digit ISO year, >> 'IW' >> > is the two-digit ISO week, but 'ID' would be the one-digit ISO >> > day-of-the-week. I'm not sure we can fix that, but I wanted to point >> > it out. >> > >> >> Is there a two digit ISO day of the week ? If not, we should use ID. As >> you say, I don't know what we can do about that. I used Brendan Jurd's >> idea, perhaps he can tell us more on this matter. >> > > Thanks for your work so far Guillaume. I agree with Peter, it is > inconsistent to have a one-digit field represented by a two-character > code. However, I don't see a way around it. 'D' is already taken to > mean the non-ISO day-of-week, and 'I' is taken to mean the last digit > of the ISO year (although to be honest I don't see where this would be > useful). > > This sort of thing is not unprecedented in to_char(). For example, > the codes 'HH24' and 'HH12' are four characters long, but resolve to a > two-digit result. 'DAY' resolves to nine characters, and so on. > > Basically I think we're stuck with ID for day-of-week and IDDD for > day-of-year. > > I will take a look at implementing 'isoyear' for extract(), and also > start putting together a patch for the documentation. If Guillaume is > still interested in adding the IDDD field to to_char(), wonderful, if > not I will pick up from his ID patch and add IDDD to it. > Sorry for the late answer. I'm still interested but, to be honest, I don't think I will have the time to do it. Perhaps in a month or so. Regards. -- Guillaume. <!-- http://abs.traduc.org/ http://lfs.traduc.org/ http://traduc.postgresqlfr.org/ -->
On 11/9/06, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Brendan Jurd a écrit : > > I will take a look at implementing 'isoyear' for extract(), and also > > start putting together a patch for the documentation. If Guillaume is > > still interested in adding the IDDD field to to_char(), wonderful, if > > not I will pick up from his ID patch and add IDDD to it. > > > > Sorry for the late answer. I'm still interested but, to be honest, I > don't think I will have the time to do it. Perhaps in a month or so. > No problem Guillaume. I'm actually nearly done adding in all these features. Thank you for getting the ball rolling!
The attached patch implements my proposal to extend support for the
ISO week date calendar.
I have added two new format fields for use with to_char, to_date and
to_timestamp:
- ID for day-of-week
- IDDD for day-of-year
This makes it possible to convert ISO week dates to and from text
fully represented in either week ('IYYY-IW-ID') or day-of-year
('IYYY-IDDD') format.
I have also added an 'isoyear' field for use with extract / date_part.
The patch includes documentation updates and some extra tests in the
regression suite for the new fields.
I have tried to implement these features with as little disruption to
the existing code as possible. I built on the existing date2iso*
functions in src/backend/utils/adt/timestamp.c, and added a few
functions of my own, but I wonder if these functions would be more
appropriately located in datetime.c, alongside date2j and j2date?
I'd also like to raise the topic of how conversion from text to ISO
week dates should be handled, where the user has specified a bogus
mixture of fields. Existing code basically ignores these issues; for
example, if a user were to call to_date('1998-01-01 2454050',
'YYYY-MM-DD J') the function returns 2006-01-01, a result of setting
the year field from YYYY, then overwriting year, month and day with
the values from the Julian date in J, then setting the month and day
normally from MM and DD.
2006-01-01 is not a valid representation of either of the values the
user specified. Now you might say "ask a silly question, get a silly
answer"; the user shouldn't send nonsense arguments to to_date and
expect a sensible result. But perhaps the right way to respond to a
broken timestamp definition is to throw an error, rather than behave
as though everything has gone to plan, and return something which is
not correct.
The same situation can arise if the user mixes ISO and Gregorian data;
how should Postgres deal with something like to_date('2006-250',
'IYYY-DDD')? The current behaviour in my patch is actually to assume
that the user meant to say 'IYYY-IDDD', since "the 250th Gregorian day
of the ISO year 2006" is total gibberish. But perhaps it should be
throwing an error message.
That's all for now, thanks for your time.
BJ
Attachment
This has been saved for the 8.3 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
---------------------------------------------------------------------------
Brendan Jurd wrote:
> The attached patch implements my proposal to extend support for the
> ISO week date calendar.
>
> I have added two new format fields for use with to_char, to_date and
> to_timestamp:
> - ID for day-of-week
> - IDDD for day-of-year
>
> This makes it possible to convert ISO week dates to and from text
> fully represented in either week ('IYYY-IW-ID') or day-of-year
> ('IYYY-IDDD') format.
>
> I have also added an 'isoyear' field for use with extract / date_part.
>
> The patch includes documentation updates and some extra tests in the
> regression suite for the new fields.
>
> I have tried to implement these features with as little disruption to
> the existing code as possible. I built on the existing date2iso*
> functions in src/backend/utils/adt/timestamp.c, and added a few
> functions of my own, but I wonder if these functions would be more
> appropriately located in datetime.c, alongside date2j and j2date?
>
> I'd also like to raise the topic of how conversion from text to ISO
> week dates should be handled, where the user has specified a bogus
> mixture of fields. Existing code basically ignores these issues; for
> example, if a user were to call to_date('1998-01-01 2454050',
> 'YYYY-MM-DD J') the function returns 2006-01-01, a result of setting
> the year field from YYYY, then overwriting year, month and day with
> the values from the Julian date in J, then setting the month and day
> normally from MM and DD.
>
> 2006-01-01 is not a valid representation of either of the values the
> user specified. Now you might say "ask a silly question, get a silly
> answer"; the user shouldn't send nonsense arguments to to_date and
> expect a sensible result. But perhaps the right way to respond to a
> broken timestamp definition is to throw an error, rather than behave
> as though everything has gone to plan, and return something which is
> not correct.
>
> The same situation can arise if the user mixes ISO and Gregorian data;
> how should Postgres deal with something like to_date('2006-250',
> 'IYYY-DDD')? The current behaviour in my patch is actually to assume
> that the user meant to say 'IYYY-IDDD', since "the 250th Gregorian day
> of the ISO year 2006" is total gibberish. But perhaps it should be
> throwing an error message.
>
> That's all for now, thanks for your time.
> BJ
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Brendan Jurd wrote:
> The attached patch implements my proposal to extend support for the
> ISO week date calendar.
>
> I have added two new format fields for use with to_char, to_date and
> to_timestamp:
> - ID for day-of-week
> - IDDD for day-of-year
>
> This makes it possible to convert ISO week dates to and from text
> fully represented in either week ('IYYY-IW-ID') or day-of-year
> ('IYYY-IDDD') format.
>
> I have also added an 'isoyear' field for use with extract / date_part.
>
> The patch includes documentation updates and some extra tests in the
> regression suite for the new fields.
>
> I have tried to implement these features with as little disruption to
> the existing code as possible. I built on the existing date2iso*
> functions in src/backend/utils/adt/timestamp.c, and added a few
> functions of my own, but I wonder if these functions would be more
> appropriately located in datetime.c, alongside date2j and j2date?
>
> I'd also like to raise the topic of how conversion from text to ISO
> week dates should be handled, where the user has specified a bogus
> mixture of fields. Existing code basically ignores these issues; for
> example, if a user were to call to_date('1998-01-01 2454050',
> 'YYYY-MM-DD J') the function returns 2006-01-01, a result of setting
> the year field from YYYY, then overwriting year, month and day with
> the values from the Julian date in J, then setting the month and day
> normally from MM and DD.
>
> 2006-01-01 is not a valid representation of either of the values the
> user specified. Now you might say "ask a silly question, get a silly
> answer"; the user shouldn't send nonsense arguments to to_date and
> expect a sensible result. But perhaps the right way to respond to a
> broken timestamp definition is to throw an error, rather than behave
> as though everything has gone to plan, and return something which is
> not correct.
>
> The same situation can arise if the user mixes ISO and Gregorian data;
> how should Postgres deal with something like to_date('2006-250',
> 'IYYY-DDD')? The current behaviour in my patch is actually to assume
> that the user meant to say 'IYYY-IDDD', since "the 250th Gregorian day
> of the ISO year 2006" is total gibberish. But perhaps it should be
> throwing an error message.
>
> That's all for now, thanks for your time.
> BJ
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Brendan Jurd wrote:
> I'd also like to raise the topic of how conversion from text to ISO
> week dates should be handled, where the user has specified a bogus
> mixture of fields. Existing code basically ignores these issues; for
> example, if a user were to call to_date('1998-01-01 2454050',
> 'YYYY-MM-DD J') the function returns 2006-01-01, a result of setting
> the year field from YYYY, then overwriting year, month and day with
> the values from the Julian date in J, then setting the month and day
> normally from MM and DD.
>
> 2006-01-01 is not a valid representation of either of the values the
> user specified. Now you might say "ask a silly question, get a silly
> answer"; the user shouldn't send nonsense arguments to to_date and
> expect a sensible result. But perhaps the right way to respond to a
> broken timestamp definition is to throw an error, rather than behave
> as though everything has gone to plan, and return something which is
> not correct.
>
> The same situation can arise if the user mixes ISO and Gregorian data;
> how should Postgres deal with something like to_date('2006-250',
> 'IYYY-DDD')? The current behaviour in my patch is actually to assume
> that the user meant to say 'IYYY-IDDD', since "the 250th Gregorian day
> of the ISO year 2006" is total gibberish. But perhaps it should be
> throwing an error message.
On these questions, we have to find out how Oracle handles it, but your
approach seems appropriate.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > On these questions, we have to find out how Oracle handles it, but > your approach seems appropriate. I don't think Oracle even has that. But personally I'd like to see errors for invalid pattern combinations. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: > Bruce Momjian wrote: > > On these questions, we have to find out how Oracle handles it, but > > your approach seems appropriate. > > I don't think Oracle even has that. But personally I'd like to see > errors for invalid pattern combinations. What do we do with other invalid pattern combinations in to_char() now? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Peter Eisentraut wrote:
>> I don't think Oracle even has that. But personally I'd like to see
>> errors for invalid pattern combinations.
> What do we do with other invalid pattern combinations in to_char() now?
Mostly, we return bogus results :-(. The formatting.c code in general
doesn't seem very robust.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Peter Eisentraut wrote: > >> I don't think Oracle even has that. But personally I'd like to see > >> errors for invalid pattern combinations. > > > What do we do with other invalid pattern combinations in to_char() now? > > Mostly, we return bogus results :-(. The formatting.c code in general > doesn't seem very robust. Yep, seems every release I am in there cleaning up some mistake repeated multiple times in the code. It needs a good cleaning. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On 2/15/07, Bruce Momjian <bruce@momjian.us> wrote: > Tom Lane wrote: > > Bruce Momjian <bruce@momjian.us> writes: > > > Peter Eisentraut wrote: > > >> I don't think Oracle even has that. But personally I'd like to see > > >> errors for invalid pattern combinations. > > > > > What do we do with other invalid pattern combinations in to_char() now? > > > > Mostly, we return bogus results :-(. The formatting.c code in general > > doesn't seem very robust. > > Yep, seems every release I am in there cleaning up some mistake repeated > multiple times in the code. It needs a good cleaning. I'm happy to volunteer to do something about the invalid field combinations, but I suspect an overhaul of formatting.c is more than I can currently chew. I figure it would be a bit misguided of me to put together a patch for invalid field combinations if somebody is about to do a rewrite of much of the code?
Brendan Jurd wrote: > On 2/15/07, Bruce Momjian <bruce@momjian.us> wrote: > > Tom Lane wrote: > > > Bruce Momjian <bruce@momjian.us> writes: > > > > Peter Eisentraut wrote: > > > >> I don't think Oracle even has that. But personally I'd like to see > > > >> errors for invalid pattern combinations. > > > > > > > What do we do with other invalid pattern combinations in to_char() now? > > > > > > Mostly, we return bogus results :-(. The formatting.c code in general > > > doesn't seem very robust. > > > > Yep, seems every release I am in there cleaning up some mistake repeated > > multiple times in the code. It needs a good cleaning. > > I'm happy to volunteer to do something about the invalid field > combinations, but I suspect an overhaul of formatting.c is more than I > can currently chew. I figure it would be a bit misguided of me to put > together a patch for invalid field combinations if somebody is about > to do a rewrite of much of the code? Yea, I was just throwing out a note that someday if someone has time, that file need a good sweeping. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > Brendan Jurd wrote: > > On 2/15/07, Bruce Momjian <bruce@momjian.us> wrote: > > > Tom Lane wrote: > > > > Bruce Momjian <bruce@momjian.us> writes: > > > > > Peter Eisentraut wrote: > > > > >> I don't think Oracle even has that. But personally I'd like to see > > > > >> errors for invalid pattern combinations. > > > > > > > > > What do we do with other invalid pattern combinations in to_char() now? > > > > > > > > Mostly, we return bogus results :-(. The formatting.c code in general > > > > doesn't seem very robust. > > > > > > Yep, seems every release I am in there cleaning up some mistake repeated > > > multiple times in the code. It needs a good cleaning. > > > > I'm happy to volunteer to do something about the invalid field > > combinations, but I suspect an overhaul of formatting.c is more than I > > can currently chew. I figure it would be a bit misguided of me to put > > together a patch for invalid field combinations if somebody is about > > to do a rewrite of much of the code? > > Yea, I was just throwing out a note that someday if someone has time, > that file need a good sweeping. Sorry, I wasn't clear. No one is currently working on overhauling formatting.c, so if you want to submit _any_ patch to improve the file, please do. :-) -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On 2/15/07, Bruce Momjian <bruce@momjian.us> wrote: > Bruce Momjian wrote: > > Brendan Jurd wrote: > > > On 2/15/07, Bruce Momjian <bruce@momjian.us> wrote: > > > > Tom Lane wrote: > > > > > Bruce Momjian <bruce@momjian.us> writes: > > > > > > Peter Eisentraut wrote: > > > > > >> I don't think Oracle even has that. But personally I'd like to see > > > > > >> errors for invalid pattern combinations. > > > > > > > > > > > What do we do with other invalid pattern combinations in to_char() now? > > > > > > > > > > Mostly, we return bogus results :-(. The formatting.c code in general > > > > > doesn't seem very robust. > > > > > > > > Yep, seems every release I am in there cleaning up some mistake repeated > > > > multiple times in the code. It needs a good cleaning. > > > > > > I'm happy to volunteer to do something about the invalid field > > > combinations, but I suspect an overhaul of formatting.c is more than I > > > can currently chew. I figure it would be a bit misguided of me to put > > > together a patch for invalid field combinations if somebody is about > > > to do a rewrite of much of the code? > > > > Yea, I was just throwing out a note that someday if someone has time, > > that file need a good sweeping. > > Sorry, I wasn't clear. No one is currently working on overhauling > formatting.c, so if you want to submit _any_ patch to improve the file, > please do. :-) It'll be a pleasure Bruce. I think it would be best to wait until the existing ISO week date patch has gone through before working on it though. Perhaps a TODO item is in order?
Brendan Jurd wrote: > On 2/15/07, Bruce Momjian <bruce@momjian.us> wrote: > > Bruce Momjian wrote: > > > Brendan Jurd wrote: > > > > On 2/15/07, Bruce Momjian <bruce@momjian.us> wrote: > > > > > Tom Lane wrote: > > > > > > Bruce Momjian <bruce@momjian.us> writes: > > > > > > > Peter Eisentraut wrote: > > > > > > >> I don't think Oracle even has that. But personally I'd like to see > > > > > > >> errors for invalid pattern combinations. > > > > > > > > > > > > > What do we do with other invalid pattern combinations in to_char() now? > > > > > > > > > > > > Mostly, we return bogus results :-(. The formatting.c code in general > > > > > > doesn't seem very robust. > > > > > > > > > > Yep, seems every release I am in there cleaning up some mistake repeated > > > > > multiple times in the code. It needs a good cleaning. > > > > > > > > I'm happy to volunteer to do something about the invalid field > > > > combinations, but I suspect an overhaul of formatting.c is more than I > > > > can currently chew. I figure it would be a bit misguided of me to put > > > > together a patch for invalid field combinations if somebody is about > > > > to do a rewrite of much of the code? > > > > > > Yea, I was just throwing out a note that someday if someone has time, > > > that file need a good sweeping. > > > > Sorry, I wasn't clear. No one is currently working on overhauling > > formatting.c, so if you want to submit _any_ patch to improve the file, > > please do. :-) > > It'll be a pleasure Bruce. I think it would be best to wait until the > existing ISO week date patch has gone through before working on it > though. OK. > Perhaps a TODO item is in order? Sure, good suggestion. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Patch applied. Thanks.
---------------------------------------------------------------------------
Brendan Jurd wrote:
> The attached patch implements my proposal to extend support for the
> ISO week date calendar.
>
> I have added two new format fields for use with to_char, to_date and
> to_timestamp:
> - ID for day-of-week
> - IDDD for day-of-year
>
> This makes it possible to convert ISO week dates to and from text
> fully represented in either week ('IYYY-IW-ID') or day-of-year
> ('IYYY-IDDD') format.
>
> I have also added an 'isoyear' field for use with extract / date_part.
>
> The patch includes documentation updates and some extra tests in the
> regression suite for the new fields.
>
> I have tried to implement these features with as little disruption to
> the existing code as possible. I built on the existing date2iso*
> functions in src/backend/utils/adt/timestamp.c, and added a few
> functions of my own, but I wonder if these functions would be more
> appropriately located in datetime.c, alongside date2j and j2date?
>
> I'd also like to raise the topic of how conversion from text to ISO
> week dates should be handled, where the user has specified a bogus
> mixture of fields. Existing code basically ignores these issues; for
> example, if a user were to call to_date('1998-01-01 2454050',
> 'YYYY-MM-DD J') the function returns 2006-01-01, a result of setting
> the year field from YYYY, then overwriting year, month and day with
> the values from the Julian date in J, then setting the month and day
> normally from MM and DD.
>
> 2006-01-01 is not a valid representation of either of the values the
> user specified. Now you might say "ask a silly question, get a silly
> answer"; the user shouldn't send nonsense arguments to to_date and
> expect a sensible result. But perhaps the right way to respond to a
> broken timestamp definition is to throw an error, rather than behave
> as though everything has gone to plan, and return something which is
> not correct.
>
> The same situation can arise if the user mixes ISO and Gregorian data;
> how should Postgres deal with something like to_date('2006-250',
> 'IYYY-DDD')? The current behaviour in my patch is actually to assume
> that the user meant to say 'IYYY-IDDD', since "the 250th Gregorian day
> of the ISO year 2006" is total gibberish. But perhaps it should be
> throwing an error message.
>
> That's all for now, thanks for your time.
> BJ
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +