ZCatalog 2.13.12 doesn't preserve relevance in search results

Bug #815469 reported by Ross Patterson
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Zope 2
Fix Released
Undecided
Hanno Schlichting

Bug Description

The change log for ZCatalog 2.13.12 says:

"Replaced `weightedIntersection` and `weightedUnion` calls with their non-weighted version, as we didn't pass in weights."

But the BTrees.Interfaces documents:

"def weightedIntersection(c1, c2, weight1=1, weight2=1)"

Meaning that by default, both results sets are given equal weight. IOW, the wieghts provided by _apply_index still have an effect when weight1 and weight2 are not provided. I have tests asserting relevance in search results order that pass under previous versions but fail under this version. Please restore the use of weightedIntersection.

Revision history for this message
Ross Patterson (rossp) wrote :

As an afterthought, it's pretty surprising that ZCatalog doesn't have test coverage on relevance sorting.

Changed in zope2:
assignee: nobody → Hanno Schlichting (hannosch)
status: New → Confirmed
Revision history for this message
Hanno Schlichting (hannosch) wrote :

Hhm, looks like I wasn't careful enough when trying to understand what weightedIntersection actually does. I tracked it down to the C code, where it boils down to:

intersection (intersection_m):

set_operation(o1, o2, 0, 0, 1, 1, 0, 1, 0);

weightedIntersection (wintersection_m):

set_operation(o1, o2, 1, 1, 1, 1, 0, 1, 0);

The fifth and sixth arguments are the weights, thus being equal. We used that in a call that basically did:

rs = None
r = _apply_index(query)
w, rs = weightedIntersection(rs, r)

and then ignored w.

So I was assuming this had no effect. But looking again, it seems that weightedIntersection is actually sneaky. It doesn't actually return a set as everyone else does, but it returns a mapping. The third and forth argument of the set_operation call are named "use_values". ZCTextIndex is the only index that returns a mapping and puts that into the apparently misnamed "resultset" variable.

I'll fix this. I've tried to improve the test coverage of ZCatalog, but there's still many glaring holes - any help is welcome.

Revision history for this message
Hanno Schlichting (hannosch) wrote :

Fix released in ZCatalog 2.13.16

Changed in zope2:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.