Thread: Error in from_char() for field 'D'?

Hey hackers,

I was doing some work in backend/utils/adt/formatting.c, and found the
following:
    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));
returnstrlen(p_inout);        }        else        {            sscanf(inout, "%1d", &tmfc->d);            return
strspace_len(inout)+ 1 + SKIP_THth(suf);        }
 

The tm_wday field is internally stored as an integer 0 - 6, with 0
being Sunday.  The 'D' formatting field, as per the documentation,
gives 1 - 7 with 1 being Sunday.  So to convert tm_wday to 'D' in
to_char(), you add one.  This works as expected.

However, in from_char(), the reverse is not true.  Looking at the code
snippet above, the digit is scanned straight into tmfc->d unaltered
(this value is later copied directly to tm->tm_wday circa line 3394).

Unless I'm missing something, when converting to text, 'D' yields 1-7,
but when converting back from text, 'D' expects 0-6.

It's not a major problem, since there's not really a use-case for
specifying dates for conversion with the 'D' field, but this behaviour
appears to be incorrect, or at the very least, incorrectly documented.

The fix should be trivial; subtract one from tmfc->d after the call to sscanf()

Regards,
BJ

"Brendan Jurd" <direvus@gmail.com> writes:
> However, in from_char(), the reverse is not true.  Looking at the code
> snippet above, the digit is scanned straight into tmfc->d unaltered
> (this value is later copied directly to tm->tm_wday circa line 3394).
> Unless I'm missing something, when converting to text, 'D' yields 1-7,
> but when converting back from text, 'D' expects 0-6.

Although this does look like a bug, I'm not sure it matters, because
AFAICS there is no code path that will look at the value of tm_wday
while constructing a timestamp value from a struct tm.  I'm inclined
not to risk messing with it just before RC1 unless a visible fault
can be demonstrated.
        regards, tom lane

This has been saved for the 8.3 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Tom Lane wrote:
> "Brendan Jurd" <direvus@gmail.com> writes:
> > However, in from_char(), the reverse is not true.  Looking at the code
> > snippet above, the digit is scanned straight into tmfc->d unaltered
> > (this value is later copied directly to tm->tm_wday circa line 3394).
> > Unless I'm missing something, when converting to text, 'D' yields 1-7,
> > but when converting back from text, 'D' expects 0-6.
> 
> Although this does look like a bug, I'm not sure it matters, because
> AFAICS there is no code path that will look at the value of tm_wday
> while constructing a timestamp value from a struct tm.  I'm inclined
> not to risk messing with it just before RC1 unless a visible fault
> can be demonstrated.
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>        choose an index scan if your joining column's datatypes do not
>        match

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +