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

From: William Morgan <wmorgan-sup@masanjin.net>
To: sup-talk <sup-talk@rubyforge.org>
Subject: Re: [sup-talk] A fix for the joining threads bug with Ferret
Date: Sun, 01 Nov 2009 14:57:44 -0800	[thread overview]
Message-ID: <1257115167-sup-6013@masanjin.net> (raw)
In-Reply-To: <1256297205-sup-683@sam.mediasupervision.de>

Reformatted excerpts from Gregor Hoffleit's message of 2009-10-23:
> 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(" ")) }

Great catch. Thank you!

> 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.

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.

> Btw, the code in xapian_index.rb looks much different. Still, I'd like
> 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 <wmorgan-sup@masanjin.net>
_______________________________________________
sup-talk mailing list
sup-talk@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-talk