Thread: text search patch status update?

Any status updates on the following patches?

1. Fragments in tsearch2 headlines:
http://archives.postgresql.org/pgsql-hackers/2008-08/msg00043.php

2. Bug in hlCover:
http://archives.postgresql.org/pgsql-hackers/2008-08/msg00089.php

-Sushant.

Sushant Sinha escribió:
> Any status updates on the following patches?
> 
> 1. Fragments in tsearch2 headlines:
> http://archives.postgresql.org/pgsql-hackers/2008-08/msg00043.php
> 
> 2. Bug in hlCover:
> http://archives.postgresql.org/pgsql-hackers/2008-08/msg00089.php

Are these ready for review?  If so, please add them to this commitfest,
http://wiki.postgresql.org/wiki/CommitFest:2008-09

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Sushant Sinha wrote:
> Patch #2. I think this is a straigt forward bug fix.

Yes, I think you're right. In hlCover(), *q is 0 when the only match is 
the first item in the text, and we shouldn't bail out with "return 
false" in that case.

But there seems to be something else going on here as well:

postgres=# select ts_headline('1 2 3 4 5', '2'::tsquery, 'MinWords=2, 
MaxWords=3'); ts_headline
-------------- <b>2</b> 3 4
(1 row)

postgres=# select ts_headline('aaa1 aaa2 aaa3 aaa4 
aaa5','aaa2'::tsquery, 'MinWords=2, MaxWords=3');   ts_headline
------------------ <b>aaa2</b> aaa3
(1 row)

In the first example, you get three words, and in the 2nd, just two. It 
must be because of the default ShortWord setting of 3. Also, if only the 
last word matches, and it's a "short word", you get the whole text:

postgres=# select ts_headline('1 2 3 4 5','5'::tsquery, 'MinWords=2, 
MaxWords=3');   ts_headline
------------------ 1 2 3 4 <b>5</b>
(1 row)

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com

I remember about that but right now I havn't time to make final review. Sorry. 
Will return soon.

Sushant Sinha wrote:
> Any status updates on the following patches?
> 
> 1. Fragments in tsearch2 headlines:
> http://archives.postgresql.org/pgsql-hackers/2008-08/msg00043.php
> 
> 2. Bug in hlCover:
> http://archives.postgresql.org/pgsql-hackers/2008-08/msg00089.php
> 
> -Sushant.
> 
> 

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
  WWW: http://www.sigaev.ru/
 

Uh, where are we on this?  I see the same output in CVS HEAD as Heikki,
and I assume he thought at least one of them was wrong.  ;-)

---------------------------------------------------------------------------

Heikki Linnakangas wrote:
> Sushant Sinha wrote:
> > Patch #2. I think this is a straigt forward bug fix.
> 
> Yes, I think you're right. In hlCover(), *q is 0 when the only match is 
> the first item in the text, and we shouldn't bail out with "return 
> false" in that case.
> 
> But there seems to be something else going on here as well:
> 
> postgres=# select ts_headline('1 2 3 4 5', '2'::tsquery, 'MinWords=2, 
> MaxWords=3');
>   ts_headline
> --------------
>   <b>2</b> 3 4
> (1 row)
> 
> postgres=# select ts_headline('aaa1 aaa2 aaa3 aaa4 
> aaa5','aaa2'::tsquery, 'MinWords=2, MaxWords=3');
>     ts_headline
> ------------------
>   <b>aaa2</b> aaa3
> (1 row)
> 
> In the first example, you get three words, and in the 2nd, just two. It 
> must be because of the default ShortWord setting of 3. Also, if only the 
> last word matches, and it's a "short word", you get the whole text:
> 
> postgres=# select ts_headline('1 2 3 4 5','5'::tsquery, 'MinWords=2, 
> MaxWords=3');
>     ts_headline
> ------------------
>   1 2 3 4 <b>5</b>
> (1 row)
> 
> -- 
>    Heikki Linnakangas
>    EnterpriseDB   http://www.enterprisedb.com
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

The default headline generation function is complicated. It checks a lot
of cases to determine the best headline to be displayed. So Heikki's
examples just say that headline generation function may not be very
intuitive. However, his examples were not affected by the bug.

Because of the bug, hlcover was not returning a cover when the query
item was the first lexeme in the text. And so the headline generation
function will return just MINWORDS rather than the actual headline as
per the logic.

After the patch you will see the difference in the example:

http://archives.postgresql.org/pgsql-hackers/2008-07/msg00785.php

-Sushant.

On Wed, 2009-01-07 at 20:50 -0500, Bruce Momjian wrote:
> Uh, where are we on this?  I see the same output in CVS HEAD as Heikki,
> and I assume he thought at least one of them was wrong.  ;-)
> 
> ---------------------------------------------------------------------------
> 
> Heikki Linnakangas wrote:
> > Sushant Sinha wrote:
> > > Patch #2. I think this is a straigt forward bug fix.
> > 
> > Yes, I think you're right. In hlCover(), *q is 0 when the only match is 
> > the first item in the text, and we shouldn't bail out with "return 
> > false" in that case.
> > 
> > But there seems to be something else going on here as well:
> > 
> > postgres=# select ts_headline('1 2 3 4 5', '2'::tsquery, 'MinWords=2, 
> > MaxWords=3');
> >   ts_headline
> > --------------
> >   <b>2</b> 3 4
> > (1 row)
> > 
> > postgres=# select ts_headline('aaa1 aaa2 aaa3 aaa4 
> > aaa5','aaa2'::tsquery, 'MinWords=2, MaxWords=3');
> >     ts_headline
> > ------------------
> >   <b>aaa2</b> aaa3
> > (1 row)
> > 
> > In the first example, you get three words, and in the 2nd, just two. It 
> > must be because of the default ShortWord setting of 3. Also, if only the 
> > last word matches, and it's a "short word", you get the whole text:
> > 
> > postgres=# select ts_headline('1 2 3 4 5','5'::tsquery, 'MinWords=2, 
> > MaxWords=3');
> >     ts_headline
> > ------------------
> >   1 2 3 4 <b>5</b>
> > (1 row)
> > 
> > -- 
> >    Heikki Linnakangas
> >    EnterpriseDB   http://www.enterprisedb.com
> > 
> > -- 
> > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-hackers
> 

Sushant Sinha wrote:
> The default headline generation function is complicated. It checks a lot
> of cases to determine the best headline to be displayed. So Heikki's
> examples just say that headline generation function may not be very
> intuitive. However, his examples were not affected by the bug.
> 
> Because of the bug, hlcover was not returning a cover when the query
> item was the first lexeme in the text. And so the headline generation
> function will return just MINWORDS rather than the actual headline as
> per the logic.
> 
> After the patch you will see the difference in the example:
> 
> http://archives.postgresql.org/pgsql-hackers/2008-07/msg00785.php

Ah, thank you for the clarification.  I now realize Heikki was more
saying the code can be improved rather than reporting a bug.  Thanks.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Sushant Sinha wrote:
>> The default headline generation function is complicated. It checks a lot
>> of cases to determine the best headline to be displayed. So Heikki's
>> examples just say that headline generation function may not be very
>> intuitive. However, his examples were not affected by the bug.
>>
>> Because of the bug, hlcover was not returning a cover when the query
>> item was the first lexeme in the text. And so the headline generation
>> function will return just MINWORDS rather than the actual headline as
>> per the logic.
>>
>> After the patch you will see the difference in the example:
>>
>> http://archives.postgresql.org/pgsql-hackers/2008-07/msg00785.php
> 
> Ah, thank you for the clarification.  I now realize Heikki was more
> saying the code can be improved rather than reporting a bug.  Thanks.

No, it does still look wrong to me.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com

OK, Heikki still believe the behavior below is a bug.  Can I get
feedback from anyone else on this?  TODO item?

---------------------------------------------------------------------------

> Heikki Linnakangas wrote:
> > Sushant Sinha wrote:
> > > Patch #2. I think this is a straigt forward bug fix.
> > 
> > Yes, I think you're right. In hlCover(), *q is 0 when the only match is 
> > the first item in the text, and we shouldn't bail out with "return 
> > false" in that case.
> > 
> > But there seems to be something else going on here as well:
> > 
> > postgres=# select ts_headline('1 2 3 4 5', '2'::tsquery, 'MinWords=2, 
> > MaxWords=3');
> >   ts_headline
> > --------------
> >   <b>2</b> 3 4
> > (1 row)
> > 
> > postgres=# select ts_headline('aaa1 aaa2 aaa3 aaa4 
> > aaa5','aaa2'::tsquery, 'MinWords=2, MaxWords=3');
> >     ts_headline
> > ------------------
> >   <b>aaa2</b> aaa3
> > (1 row)
> > 
> > In the first example, you get three words, and in the 2nd, just two. It 
> > must be because of the default ShortWord setting of 3. Also, if only the 
> > last word matches, and it's a "short word", you get the whole text:
> > 
> > postgres=# select ts_headline('1 2 3 4 5','5'::tsquery, 'MinWords=2, 
> > MaxWords=3');
> >     ts_headline
> > ------------------
> >   1 2 3 4 <b>5</b>
> > (1 row)
> > 
> > -- 
> >    Heikki Linnakangas
> >    EnterpriseDB   http://www.enterprisedb.com
> > 
> > -- 
> > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-hackers
> 
> -- 
>   Bruce Momjian  <bruce@momjian.us>        http://momjian.us
>   EnterpriseDB                             http://enterprisedb.com
> 
>   + If your life is a hard drive, Christ can be your backup. +
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +