Re: tsearch2 regression test failures

tsearch2 regression tests are also failing on win32/msvc, with attached
diffs.

Any pointers on where to start? ;)

//Magnus

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> tsearch2 regression tests are also failing on win32/msvc, with attached
> diffs.
> Any pointers on where to start? ;)

FWIW, it looks like it failed to reject stopwords. Is it possible you
ran it in an environment that would make it pick the Russian stopword
list?

regards, tom lane

> FWIW, it looks like it failed to reject stopwords. Is it possible you
Right.

I suppose the problem is with '\r\n'... Try attached patch.
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/

On Mon, Mar 26, 2007 at 02:32:26PM +0400, Teodor Sigaev wrote:
> >FWIW, it looks like it failed to reject stopwords. Is it possible you
> Right.
>
> I suppose the problem is with '\r\n'... Try attached patch.
> --
> Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
> WWW:
> http://www.sigaev.ru/

> *** ./contrib/tsearch2/stopword.c.orig Mon Mar 26 14:25:16 2007
> --- ./contrib/tsearch2/stopword.c Mon Mar 26 14:28:25 2007
> ***************
> *** 47,53 ****
>
> while (fgets(buf, sizeof(buf), hin))
> {
> ! buf[strlen(buf) - 1] = '\0';
> pg_verifymbstr(buf, strlen(buf), false);
> if (*buf == '\0')
> continue;
> --- 47,57 ----
>
> while (fgets(buf, sizeof(buf), hin))
> {
> ! pbuf = buf;
> ! while( !isspace( *pbuf ) )
> ! pbuf++;
> ! *pbuf = '\0';
> !
> pg_verifymbstr(buf, strlen(buf), false);
> if (*buf == '\0')
> continue;

Yup, that solved the problem, thanks.

Wouldn't it be more efficiently written to walk the string backwards until
!isspace instead? Not sure that it matters at all, but then you'll
normallyi never step over more than two bytes...

//Magnus

> Yup, that solved the problem, thanks.
I'll commit extended patch - there is one more place with the same bug.

> Wouldn't it be more efficiently written to walk the string backwards until
> !isspace instead? Not sure that it matters at all, but then you'll
> normallyi never step over more than two bytes...
It doesn't significant matter - file reads once per backend lifetime.

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/

> > Yup, that solved the problem, thanks.> I'll commit extended patch - there is one more place with the same bug.

Ok, thanks.

> Wouldn't it be more efficiently written to walk the string backwards until
> > !isspace instead? Not sure that it matters at all, but then you'll
> > normallyi never step over more than two bytes...
> It doesn't significant matter - file reads once per backend lifetime.
>

ok.

/Magnus

Teodor Sigaev <teodor(at)sigaev(dot)ru> writes:
> ! pbuf = buf;
> ! while( !isspace( *pbuf ) )
> ! pbuf++;
> ! *pbuf = '\0';

Surely the loop needs to look like

while (*pbuf && !isspace(*pbuf))
pbuf++;

regards, tom lane

Tom Lane wrote:
> Teodor Sigaev <teodor(at)sigaev(dot)ru> writes:
>
>> ! pbuf = buf;
>> ! while( !isspace( *pbuf ) )
>> ! pbuf++;
>> ! *pbuf = '\0';
>>
>
> Surely the loop needs to look like
>
> while (*pbuf && !isspace(*pbuf))
> pbuf++;
>
>
Yes.

But in any case, I am having difficulty in understanding why we are
seeing a CR at all - the file should be opened in text mode, which
should translate CR-LF in the file to a simple LF in the buffer. So
regardless of the odd behavior of CVSNT, which presumabye caused this
mess, it's rather strange.

Can someone please explain?

cheers

andrew