Subject: Re: [sup-talk] A fix for the joining threads bug with Ferret

From: Tero Tilus <[email-masked]>
Date: Fri, 23 Oct 2009 16:15:50 +0300

Excerpts from Gregor Hoffleit's message:
> With my limited Ruby knowledge, my quick and dirty fix was:
> 
>     if entry[:refs]!="" then
>        d[:refs]=entry[:refs]
>     else
>        d[:refs]=(m.refs + m.replytos).uniq.join(" ")
>     end
> 
> Is this what the above code is about?

I would expect it to be something alike.  Rails has Object#empty?
which is true (I think) for nil, false, empty list and empty string.
I'd go with

  entry[:refs].to_s != ""

to handle nil as "no refs" too.

-- 
Tero Tilus ## 050 3635 235 ## http://tero.tilus.net/
_______________________________________________
sup-talk mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-talk

---

From: Gregor Hoffleit <[email-masked]>
Date: Fri, 23 Oct 2009 13:48:31 +0200

I did a little bit research regarding the problem that joining threads
isn't persistent (as described in [1]-[3]).

I managed to track down the problem until the following line in
FerretIndex#sync_message in lib/sup/ferret_index.rb.

  d = { ...  :refs => (entry[:refs] || (m.refs + m.replytos).uniq.join(" ")) }

I have problems to understand what this line is supposed to do.

For me, it always evaluates to "entry[:refs]" (even if that's an empty
string!), losing the reference in the modified message m, which was added
by add_ref. Therefore the manual join is always lost.

    if entry[:refs]!="" then
       d[:refs]=entry[:refs]
    else

Btw, the code in xapian_index.rb looks much different. Still, I'd like
to see this fixed for Xapian.

Regards,
    Gregor Hoffleit

[1] http://sup.rubyforge.org/ditz/issue-4e501973cea5bd1f28739ae4cea98edce8249895.html
[2] http://rubyforge.org/pipermail/sup-talk/2009-April/002050.html
[3] http://rubyforge.org/pipermail/sup-talk/2009-April/002060.html
_______________________________________________
sup-talk mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-talk

---

From: William Morgan <[email-masked]>
Date: Sun, 01 Nov 2009 14:57:44 -0800

>   d = { ...  :refs => (entry[:refs] || (m.refs + m.replytos).uniq.join(" ")) }

Great catch. Thank you!

> string!), losing the reference in the modified message m, which was
> added by add_ref. Therefore the manual join is always lost.

Yep, that's the problem. In general all those fields are ||= in order to
save the work of computing them again when we're updating a message
(because the way Ferret works is we have to rewrite the entire record
every time we change the labels, so we want that to be as fast as
possible). But for certain fields, including refs, we should regenerate
them because that data can change.

> to see this fixed for Xapian.

I believe that the fix for both is simply to set the entry[:refs] every
time, and not bother with ||=. (In fact I think the empty string check
won't work for threads that already have refs set, i.e. joining threads
of size >1.)

I've put this fix on branch thread-joining-fix and merged into next.
Check it out.
-- 
William <[email-masked]>
_______________________________________________
sup-talk mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-talk