# I'm sorry for my late response.

I confirmed that the error of regression is caused by my code inserting 
setlocale() into ecpglib of local branch.
No other tests occur error in non-C locale.

The following is about other topics.


1. About regression test

We should test the followings:
- PGTYPEStimestamp_from_asc("1994-02-11 26:10:35", NULL) returns 0.
- PGTYPEStimestamp_fmt_asc() can accept format string including %x and %X.

ecpglib should be affected by only setlocale() called by user application and
dt_test.pgc does not call it. So the following test is the best, I think.
Please see attached patch for detail (fix_pgtypeslib_regress.patch).

    ts1 = PGTYPEStimestamp_from_asc("1994-02-11 3:10:35", NULL);
    text = PGTYPEStimestamp_to_asc(ts1);
    printf("timestamp_to_asc2: %s\n", text);
    PGTYPESchar_free(text);

/*  abc-03:10:35-def-02/11/94-gh  */
/*      12345678901234567890123456789 */

    out = (char*) malloc(32);
    i = PGTYPEStimestamp_fmt_asc(&ts1, out, 31, "abc-%X-def-%x-ghi%%");
    printf("timestamp_fmt_asc: %d: %s\n", i, out);
    free(out);

    ts1 = PGTYPEStimestamp_from_asc("1994-02-11 26:10:35", NULL);
    text = PGTYPEStimestamp_to_asc(ts1);
    printf("timestamp_to_asc3: %s\n", text);
    PGTYPESchar_free(text);

We should also add tests that check PGTYPEStimestamp_*() set errno for invalid 
input correctly,
but I want to leave the improvement to the next timing when implement for 
timestamp is changed.
(Maybe the timing will not come.)


2. About document of PGTYPEStimestamp_from_asc() and PGTYPESInvalidTimestamp

0 returned by PGTYPEStimestamp_from_asc () is a valid timestamp as you 
commented and
we should not break compatibility.
So we should remove the document for PGTYPESInvalidTimestamp and add one for 
checking errno
to description of PGTYPEStimestamp_from_asc().
Please see attached patch for detail (fix_PGTYPESInvalidTimestamp_doc.patch).


3. About endptr of *_from_asc()
> PGTYPESdate_from_asc    (ParseDate)
> PGTYPEStimestamp_from_asc    (ParseDate)
> PGTYPESinterval_from_asc    (ParseDate)
> PGTYPESnumeric_from_asc

Basically, they return immediately just after detecting invalid format.
However, after passing the narrow parse, they could fails (e.g. failure of 
DecodeInterval(), DecodeISO8601Interval(), malloc(), and so on).

So we should write as follows:
   If the function detects invalid format,
   then it stores the address of the first invalid character in
   endptr. However, don't assume it successed if
   endptr points to end of input because other
   processing(e.g. memory allocation) could fails.
   Therefore, you should check return value and errno for detecting error.
   You can safely endptr to NULL.

I also found pandora box that description of the followings don't show their 
behavior when it fails.
I fix doc including them. Please see attached 
patch(fix_pgtypeslib_funcs_docs.patch).
- PGTYPESdate_from_asc()        # sets errno. (can not check return value)
- PGTYPESdate_defmt_asc()       # returns -1 and sets errno
- PGTYPEStimestamp_to_asc()     # returns NULL and sets errno
- PGTYPEStimestamp_defmt_asc()  # just returns 1 and doesn't set errno!
- PGTYPESinterval_new()         # returns NULL and sets errno
- PGTYPESinterval_from_asc()    # returns NULL and sets errno
- PGTYPESinterval_to_asc()      # returns NULL and sets errno
- PGTYPESinterval_copy          # currently always return 0
- PGTYPESdecimal_new()          # returns NULL and sets errno


4. Bug of PGTYPEStimestamp_defmt_asc()
PGTYPEStimestamp_defmt_asc() doesn't set errno on failure.
I didn't make a patch for it yet.

Best Regards
Ryo Matsumura

Attachment: fix_pgtypeslib_regress.patch
Description: fix_pgtypeslib_regress.patch

Attachment: fix_pgtypeslib_funcs_docs.patch
Description: fix_pgtypeslib_funcs_docs.patch

Attachment: fix_PGTYPESInvalidTimestamp_doc.patch
Description: fix_PGTYPESInvalidTimestamp_doc.patch

Reply via email to