Over-rigidity in recent to_timestamp() rewrite

Started by Tom Lanealmost 17 years ago4 messages
#1Tom Lane
tgl@sss.pgh.pa.us

Whilst poking at bug #4702 I noticed that PG CVS HEAD rejects use of
AD/BC notation, as well as CC (separate century) fields, in combination
with ISO-style day numbers. I don't see the point of this. It's
historically inaccurate, no doubt, but so is use of Gregorian counting.
So I suggest the attached fix. Does this make anyone unhappy?

regards, tom lane

Index: src/backend/utils/adt/formatting.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v
retrieving revision 1.155
diff -c -r1.155 formatting.c
*** src/backend/utils/adt/formatting.c	12 Mar 2009 00:53:25 -0000	1.155
--- src/backend/utils/adt/formatting.c	14 Mar 2009 17:37:05 -0000
***************
*** 709,721 ****
   */
  static const KeyWord DCH_keywords[] = {
  /*	name, len, id, is_digit, date_mode */
! 	{"A.D.", 4, DCH_A_D, FALSE, FROM_CHAR_DATE_GREGORIAN},	/* A */
  	{"A.M.", 4, DCH_A_M, FALSE, FROM_CHAR_DATE_NONE},
! 	{"AD", 2, DCH_AD, FALSE, FROM_CHAR_DATE_GREGORIAN},
  	{"AM", 2, DCH_AM, FALSE, FROM_CHAR_DATE_NONE},
! 	{"B.C.", 4, DCH_B_C, FALSE, FROM_CHAR_DATE_GREGORIAN},	/* B */
! 	{"BC", 2, DCH_BC, FALSE, FROM_CHAR_DATE_GREGORIAN},
! 	{"CC", 2, DCH_CC, TRUE, FROM_CHAR_DATE_GREGORIAN},		/* C */
  	{"DAY", 3, DCH_DAY, FALSE, FROM_CHAR_DATE_NONE},		/* D */
  	{"DDD", 3, DCH_DDD, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"DD", 2, DCH_DD, TRUE, FROM_CHAR_DATE_GREGORIAN},
--- 709,721 ----
   */
  static const KeyWord DCH_keywords[] = {
  /*	name, len, id, is_digit, date_mode */
! 	{"A.D.", 4, DCH_A_D, FALSE, FROM_CHAR_DATE_NONE},		/* A */
  	{"A.M.", 4, DCH_A_M, FALSE, FROM_CHAR_DATE_NONE},
! 	{"AD", 2, DCH_AD, FALSE, FROM_CHAR_DATE_NONE},
  	{"AM", 2, DCH_AM, FALSE, FROM_CHAR_DATE_NONE},
! 	{"B.C.", 4, DCH_B_C, FALSE, FROM_CHAR_DATE_NONE},		/* B */
! 	{"BC", 2, DCH_BC, FALSE, FROM_CHAR_DATE_NONE},
! 	{"CC", 2, DCH_CC, TRUE, FROM_CHAR_DATE_NONE},			/* C */
  	{"DAY", 3, DCH_DAY, FALSE, FROM_CHAR_DATE_NONE},		/* D */
  	{"DDD", 3, DCH_DDD, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"DD", 2, DCH_DD, TRUE, FROM_CHAR_DATE_GREGORIAN},
***************
*** 757,769 ****
  	{"YYY", 3, DCH_YYY, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"YY", 2, DCH_YY, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"Y", 1, DCH_Y, TRUE, FROM_CHAR_DATE_GREGORIAN},
! 	{"a.d.", 4, DCH_a_d, FALSE, FROM_CHAR_DATE_GREGORIAN},	/* a */
  	{"a.m.", 4, DCH_a_m, FALSE, FROM_CHAR_DATE_NONE},
! 	{"ad", 2, DCH_ad, FALSE, FROM_CHAR_DATE_GREGORIAN},
  	{"am", 2, DCH_am, FALSE, FROM_CHAR_DATE_NONE},
! 	{"b.c.", 4, DCH_b_c, FALSE, FROM_CHAR_DATE_GREGORIAN},	/* b */
! 	{"bc", 2, DCH_bc, FALSE, FROM_CHAR_DATE_GREGORIAN},
! 	{"cc", 2, DCH_CC, TRUE, FROM_CHAR_DATE_GREGORIAN},		/* c */
  	{"day", 3, DCH_day, FALSE, FROM_CHAR_DATE_NONE},		/* d */
  	{"ddd", 3, DCH_DDD, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"dd", 2, DCH_DD, TRUE, FROM_CHAR_DATE_GREGORIAN},
--- 757,769 ----
  	{"YYY", 3, DCH_YYY, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"YY", 2, DCH_YY, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"Y", 1, DCH_Y, TRUE, FROM_CHAR_DATE_GREGORIAN},
! 	{"a.d.", 4, DCH_a_d, FALSE, FROM_CHAR_DATE_NONE},		/* a */
  	{"a.m.", 4, DCH_a_m, FALSE, FROM_CHAR_DATE_NONE},
! 	{"ad", 2, DCH_ad, FALSE, FROM_CHAR_DATE_NONE},
  	{"am", 2, DCH_am, FALSE, FROM_CHAR_DATE_NONE},
! 	{"b.c.", 4, DCH_b_c, FALSE, FROM_CHAR_DATE_NONE},		/* b */
! 	{"bc", 2, DCH_bc, FALSE, FROM_CHAR_DATE_NONE},
! 	{"cc", 2, DCH_CC, TRUE, FROM_CHAR_DATE_NONE},			/* c */
  	{"day", 3, DCH_day, FALSE, FROM_CHAR_DATE_NONE},		/* d */
  	{"ddd", 3, DCH_DDD, TRUE, FROM_CHAR_DATE_GREGORIAN},
  	{"dd", 2, DCH_DD, TRUE, FROM_CHAR_DATE_GREGORIAN},
#2David Fetter
david@fetter.org
In reply to: Tom Lane (#1)
Re: Over-rigidity in recent to_timestamp() rewrite

On Sat, Mar 14, 2009 at 01:39:35PM -0400, Tom Lane wrote:

Whilst poking at bug #4702 I noticed that PG CVS HEAD rejects use of
AD/BC notation, as well as CC (separate century) fields, in
combination with ISO-style day numbers. I don't see the point of
this. It's historically inaccurate, no doubt, but so is use of
Gregorian counting. So I suggest the attached fix. Does this make
anyone unhappy?

Works for me :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

#3Brendan Jurd
direvus@gmail.com
In reply to: Tom Lane (#1)
Re: Over-rigidity in recent to_timestamp() rewrite

On Sun, Mar 15, 2009 at 4:39 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Whilst poking at bug #4702 I noticed that PG CVS HEAD rejects use of
AD/BC notation, as well as CC (separate century) fields, in combination
with ISO-style day numbers.  I don't see the point of this.  It's
historically inaccurate, no doubt, but so is use of Gregorian counting.
So I suggest the attached fix.  Does this make anyone unhappy?

I don't have any technical problem with using CC to specify the
century separately ... although I do wonder why anybody would want to
do so.

Apparently ISO 8601 isn't explicit about how years earlier than 1 AD
might be handled, because the standard is only designed to support
dates going back as far as 1582. However, the basis for "week date"
years is supposed to be the same as that for Gregorian years, so I
guess BC could work the same way in both systems.

Short version: I think using CC and AD/BC in combination with week
dates would be downright weird, but I don't object to the patch.

Cheers,
BJ

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Brendan Jurd (#3)
Re: Over-rigidity in recent to_timestamp() rewrite

Brendan Jurd <direvus@gmail.com> writes:

Short version: I think using CC and AD/BC in combination with week
dates would be downright weird, but I don't object to the patch.

I agree it's pretty weird, but I can't immediately see any reason that
it shouldn't (be allowed to) work. It would only get interesting if
you want to posit that ISO years shouldn't be based on the Gregorian
calendar that far back.

Some experimentation shows that it doesn't work, or at least doesn't
give sane results, in pre-8.4 branches; I have not traced the code to
make sure but I think this is because of the other bug I noted with
passing the wrong year variable to the isoweek code. I don't think
that's important enough to back-patch but it is worth getting it right
going forward.

regards, tom lane