Archive of RubyForge sup-talk mailing list
 help / color / mirror / Atom feed
* [sup-talk] A fix for the joining threads bug with Ferret
@ 2009-10-23 11:48 Gregor Hoffleit
  2009-10-23 13:15 ` Tero Tilus
  2009-11-01 22:57 ` William Morgan
  0 siblings, 2 replies; 3+ messages in thread
From: Gregor Hoffleit @ 2009-10-23 11:48 UTC (permalink / raw)
  To: sup-talk

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.

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?


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
sup-talk@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-talk


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [sup-talk] A fix for the joining threads bug with Ferret
  2009-10-23 11:48 [sup-talk] A fix for the joining threads bug with Ferret Gregor Hoffleit
@ 2009-10-23 13:15 ` Tero Tilus
  2009-11-01 22:57 ` William Morgan
  1 sibling, 0 replies; 3+ messages in thread
From: Tero Tilus @ 2009-10-23 13:15 UTC (permalink / raw)
  To: sup-talk

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
sup-talk@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-talk


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [sup-talk] A fix for the joining threads bug with Ferret
  2009-10-23 11:48 [sup-talk] A fix for the joining threads bug with Ferret Gregor Hoffleit
  2009-10-23 13:15 ` Tero Tilus
@ 2009-11-01 22:57 ` William Morgan
  1 sibling, 0 replies; 3+ messages in thread
From: William Morgan @ 2009-11-01 22:57 UTC (permalink / raw)
  To: sup-talk

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-11-01 23:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-23 11:48 [sup-talk] A fix for the joining threads bug with Ferret Gregor Hoffleit
2009-10-23 13:15 ` Tero Tilus
2009-11-01 22:57 ` William Morgan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox