Re: Bug: PGTYPEStimestamp_from_asc() in ECPG pgtypelib

"Ryo Matsumura (Fujitsu)" <matsumura(dot)ryo(at)fujitsu(dot)com> writes:
>> But how come we haven't noticed that
>> before? Have you added a setlocale() call somewhere?

> I didn't notice to this point.
> I added setlocale() to ECPG in my local branch.
> I will test again after removing it.
> It looks to me like existing ECPG code does not include setlocale().

> So Please ignore about behavior of PGTYPEStimestamp_fmt_asc().

If that's the only ecpg test case that fails under a non-C locale,
I think it'd be good to change it to use a non-locale-dependent
format string. Surely there's no compelling reason why it has to
use %x/%X rather than something more stable.

> I want to continue to discuss about PGTYPEStimestamp_from_asc.
> Current PGTYPEStimestamp_from_asc() returns 0, but should we return -1?

No, I think changing its behavior now after so many years would be a
bad idea. In any case, what's better about -1? They are both legal
timestamp values. I think we'd better fix the docs to match what the
code actually does, and to tell people that they *must* check errno
to detect errors.

>> I'm a bit inclined to just drop "block 3".

> Are you concerned at the above point?

Given your point that no other dt_test case is checking error
behavior, I'm not too concerned about dropping this one. If
we wanted to take the issue seriously, we'd need to add a bunch
of new tests not just tweak this one. (It's far from obvious that
this was even meant to be a test of an error case --- it looks like
just sloppily-verified testing to me. "block 3" must have been
dropped in after the tests before and after it were written,
without noticing that it changed their results.)

regards, tom lane