Skip site navigation (1) Skip section navigation (2)

Re: to_date() validation

From: "Alex Hunsaker" <badalex(at)gmail(dot)com>
To: "Brendan Jurd" <direvus(at)gmail(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: to_date() validation
Date: 2008-09-06 19:58:20
Message-ID: 34d269d40809061258t46f90218r3152814dd18880ab@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
On Fri, Aug 29, 2008 at 7:39 PM, Brendan Jurd <direvus(at)gmail(dot)com> wrote:
> Hi all,
>
> Well, it's been a long time coming, but I've finally got a patch to
> improve the error handling of to_date() and to_timestamp(), as
> previously discussed on -hackers. [1][2]

BTW -patches is obsolete just submit pathces to -hackers.

Im just following this:
http://wiki.postgresql.org/wiki/Reviewing_a_Patch so lets get started.

Submission review: Everything looks good. Tests seem reasonable patch
applies etc.

Usability review: I think both linked threads in the parent mail give
sufficient justification.

Feature test: Everything seems to work as advertised. However before
via sscanf() most limited the max length of the input.  Before they
were just silently ignored now they get added to the result:

patched:
# SELECT to_timestamp('11111', 'HHMM');
         to_timestamp
------------------------------
 0009-03-19 11:00:00-06:59:56

8.3.3:
# SELECT to_timestamp('11111', 'HHMM');
       to_timestamp
---------------------------
 0001-11-01 11:00:00-07 BC

Performance review: simple pgbench of to_timestamp did not show any
noticeable differences

Coding review: seemed fine to me, code matched surrounding style,
comments made sense etc

Code review: one minor nit
*** a/src/backend/utils/adt/formatting.c
--- b/src/backend/utils/adt/formatting.c
***************
*** 781,787 **** static const KeyWord DCH_keywords[] = {
  	{"y", 1, DCH_Y, TRUE, FROM_CHAR_DATE_GREGORIAN},

  	/* last */
! 	{NULL, 0, 0, 0}
  };

  /* ----------
--- 781,787 ----
  	{"y", 1, DCH_Y, TRUE, FROM_CHAR_DATE_GREGORIAN},

  	/* last */
! 	{NULL, 0, 0, 0, 0}
  };

  /* ----------
***************

In response to

Responses

pgsql-hackers by date

Next:From: Bruce MomjianDate: 2008-09-06 20:10:57
Subject: Re: Need more reviewers!
Previous:From: Asko OjaDate: 2008-09-06 19:45:13
Subject: Re: reducing statistics write overhead

pgsql-patches by date

Next:From: Jaime CasanovaDate: 2008-09-06 20:41:29
Subject: Re: [PgFoundry] Unsigned Data Types [1 of 2]
Previous:From: Bruce MomjianDate: 2008-09-06 19:55:21
Subject: Re: still alive?

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group