From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "Ryo Matsumura (Fujitsu)" <matsumura(dot)ryo(at)fujitsu(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Bug: PGTYPEStimestamp_from_asc() in ECPG pgtypelib |
Date: | 2024-05-09 19:39:03 |
Message-ID: | 1317899.1715283543@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
"Ryo Matsumura (Fujitsu)" <matsumura(dot)ryo(at)fujitsu(dot)com> writes:
> It is indefinite what PGTYPEStimestamp_from_asc() returns in error.
> The following is written in document(36.6.8. Special Constants of pgtypeslib):
> A value of type timestamp representing an invalid time stamp.
> This is returned by the function PGTYPEStimestamp_from_asc on parse error.
> However, PGTYPESInvalidTimestamp is not defined anywhere.
Ugh.
> At current implementation, PGTYPEStimestamp_from_asc returns -1.
It looks to me like it returns 0 ("noresult"). Where are you seeing
-1?
That documentation has more problems too: it claims that "endptr"
is unimplemented, which looks like a lie to me: the code is there
to do it, and there are several callers that depend on it.
> Regression test of pgtypelib is not robust (maybe incorrect).
> Our test failed although there is no bug actually.
> I think block2 and block3 should be swapped.
Hm, block3 seems to be completely nuts. It looks like the code is
rejecting the input (probably because "26" is out of range) and
returning zero, because what we see in the expected-stdout file is:
timestamp_to_asc3: 2000-01-01 00:00:00
We also see that the expected output from the PGTYPEStimestamp_fmt_asc
step is:
timestamp_fmt_asc: 0: abc-00:00:00-def-01/01/00-ghi%
which is consistent with that, but not very much like what the
comment is expecting. I'm a bit inclined to just drop "block 3".
If we want to keep some kind of test of the error behavior,
it doesn't belong right there, and it should be showing what errno
gets set to.
As for the "lack of robustness", I'll bet the problem you are
seeing is that the test uses the %X/%x format specifiers which
are locale-dependent. But how come we haven't noticed that
before? Have you added a setlocale() call somewhere?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2024-05-09 19:39:20 | Re: request for database identifier in the startup packet |
Previous Message | Dagfinn Ilmari Mannsåker | 2024-05-09 19:38:24 | Re: open items |