Re: to_timestamp not stable if date string shorter than

On Tue, 2 Sep 2003, Stephan Szabo wrote:

On Tue, 2 Sep 2003, Stephan Szabo wrote:

On Tue, 2 Sep 2003, Tom Lane wrote:

"Stacy White" <harsh@computer.org> writes:

to_timestamp appears to pick up the time-of-day from the previous call's
return value if a date string has no time component. For example:

Weird. I do not see that here, on either 7.3.4 or current sources.
Can anyone else reproduce it?

From my beta1 machine it looks to me that to_timestamp is willing to read

off the end of the input string sometimes:

Breakpoint 3, DCH_processor (node=0x82d8da0, inout=0x8347860 "2003-06",
flag=2, data=0xbfffd080) at formatting.c:1302
1302 len = n->key->action(n->key->id, s,
n->suffix, flag, n, data);
(gdb) print s
$3 = 0x8347860 "2003-06"
(gdb) cont
Continuing.

Breakpoint 3, DCH_processor (node=0x82d8da0, inout=0x8347860 "2003-06",
flag=2, data=0xbfffd080) at formatting.c:1302
1302 len = n->key->action(n->key->id, s,
n->suffix, flag, n, data);
(gdb) print s
$4 = 0x8347865 "06"
(gdb) cont
Continuing.

** All of a sudden at this following breakpoint s has more text in it and
is past the \0 which would be at 0x8347867 AFAICS. ***

Breakpoint 3, DCH_processor (node=0x82d8da0, inout=0x8347860 "2003-06",
flag=2, data=0xbfffd080) at formatting.c:1302
1302 len = n->key->action(n->key->id, s,
n->suffix, flag, n, data);
(gdb) print s
$5 = 0x8347868 "04 02:02:02"
(gdb) cont
Continuing.

Breakpoint 3, DCH_processor (node=0x82d8da0, inout=0x8347860 "2003-06",
flag=2, data=0xbfffd080) at formatting.c:1302
1302 len = n->key->action(n->key->id, s,
n->suffix, flag, n, data);
(gdb) print s
$6 = 0x834786b "02:02:02"
(gdb)

---
I don't entirely understand all of what that code is doing, but I think
there's something in there that needs to get fixed.

Replying to myself again:
In DCH_processor (formatting.c), it doesn't seem to stop if it's in the
middle of processing nodes but runs off the inout string, should the for
loop be something like:
for (n=node,s=inout;n->type!=NODE_TYPE_END && *s!='\0';++n,++s) {
and get rid of the ++s at the bottom of the loop for safety?

Stephan Szabo <sszabo@megazone.bigpanda.com> writes:

I don't entirely understand all of what that code is doing, but I think
there's something in there that needs to get fixed.

Oh-ho, this is interesting:

Build CVS tip on RHL 8.0 with --enable-cassert: no bug.

Build CVS tip on RHL 8.0 without --enable-cassert: bug.

Digging ...

regards, tom lane

Stephan Szabo <sszabo@megazone.bigpanda.com> writes:

Replying to myself again:
In DCH_processor (formatting.c), it doesn't seem to stop if it's in the
middle of processing nodes but runs off the inout string, should the for
loop be something like:
for (n=node,s=inout;n->type!=NODE_TYPE_END && *s!='\0';++n,++s) {
and get rid of the ++s at the bottom of the loop for safety?

That wouldn't change the behavior, would it?

The code is definitely running off the end of the input string. I am
tempted to suggest that the "++s" at the bottom of the loop should
become
if (*s)
++s;
but I'm not sure enough of the intentions of this code to recommend
that as a full fix. Karel, the ball's in your court ...

regards, tom lane

On Wed, 3 Sep 2003, Tom Lane wrote:

Stephan Szabo <sszabo@megazone.bigpanda.com> writes:

Replying to myself again:
In DCH_processor (formatting.c), it doesn't seem to stop if it's in the
middle of processing nodes but runs off the inout string, should the for
loop be something like:
for (n=node,s=inout;n->type!=NODE_TYPE_END && *s!='\0';++n,++s) {
and get rid of the ++s at the bottom of the loop for safety?

That wouldn't change the behavior, would it?

It would I believe, as soon as it reached a \0 it'd stop the loop, but
checking *s is probably the wrong thing to do when flag==TO_CHAR, so I
think that bailing to Karel is probably the right choice.

On Wed, Sep 03, 2003 at 12:03:59AM -0400, Tom Lane wrote:

Stephan Szabo <sszabo@megazone.bigpanda.com> writes:

I don't entirely understand all of what that code is doing, but I think
there's something in there that needs to get fixed.

Oh-ho, this is interesting:

Build CVS tip on RHL 8.0 with --enable-cassert: no bug.

Build CVS tip on RHL 8.0 without --enable-cassert: bug.

It's seems like typical leak... I will try fix it today.

Karel

--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/

On Wed, Sep 03, 2003 at 12:15:47AM -0400, Tom Lane wrote:

that as a full fix. Karel, the ball's in your court ...

Fixed. All tests passed. My court is without ball now. The patch is
attached.

Thanks, it was good find!

Karel

--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/

Karel Zak <zakkr@zf.jcu.cz> writes:

Fixed. All tests passed. My court is without ball now. The patch is
attached.

Applied to CVS HEAD. I also applied just the

+ if (flag == FROM_CHAR && *s=='\0')
+ break;

part to the 7.3 branch --- the rest of the changes didn't apply cleanly,
and seemed unrelated anyway. This fixes the immediate complaint, but
if there's some reason to apply the rest to 7.3.*, would you submit
an additional patch against that branch?

regards, tom lane

On Wed, Sep 03, 2003 at 11:10:33AM -0400, Tom Lane wrote:

Karel Zak <zakkr@zf.jcu.cz> writes:

Fixed. All tests passed. My court is without ball now. The patch is
attached.

Applied to CVS HEAD. I also applied just the

+ if (flag == FROM_CHAR && *s=='\0')
+ break;

part to the 7.3 branch --- the rest of the changes didn't apply cleanly,
and seemed unrelated anyway. This fixes the immediate complaint, but

Hmm.. but the patch fix another small bug which I found today.
Please, if you don't submit all of the patch, fix two lines in
do_to_timestamp():

line: 2997 must be:
incache = TRUE;

line: 3039 must be:
if (!incache)

(Note: this another bug appear if format picture for date/time is
greater than 128 bytes -- for this probably nobody find it yet)

if there's some reason to apply the rest to 7.3.*, would you submit
an additional patch against that branch?

Yep.

Karel

--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/

Karel Zak <zakkr@zf.jcu.cz> writes:

Hmm.. but the patch fix another small bug which I found today.

Okay. Please send a patch against 7.3 tip and I'll throw it in.

regards, tom lane

On Wed, Sep 03, 2003 at 11:53:35AM -0400, Tom Lane wrote:

Karel Zak <zakkr@zf.jcu.cz> writes:

Hmm.. but the patch fix another small bug which I found today.

Okay. Please send a patch against 7.3 tip and I'll throw it in.

The patch for REL7_3_4 is attached.

Karel

--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/

Karel Zak <zakkr@zf.jcu.cz> writes:

Hmm.. but the patch fix another small bug which I found today.

Okay. Please send a patch against 7.3 tip and I'll throw it in.

The patch for REL7_3_4 is attached.

Applied, thanks.

regards, tom lane