Re: Bug: PGTYPEStimestamp_from_asc() in ECPG pgtypelib

# 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