Fix a regression that caused incorrect results when searching on numeric or date fields with relational operators. Fixes #1423390 [Numeric search not working for rating](https://bugs.launchpad.net/calibre/+bug/1423390)

Due to python dictionary randomization, which caused the operators to be
tried in random order
This commit is contained in:
Kovid Goyal 2015-02-19 09:24:20 +05:30
parent 9dee96270b
commit bd5ba883a1
2 changed files with 42 additions and 32 deletions

View File

@ -7,10 +7,10 @@ __license__ = 'GPL v3'
__copyright__ = '2013, Kovid Goyal <kovid at kovidgoyal.net>' __copyright__ = '2013, Kovid Goyal <kovid at kovidgoyal.net>'
__docformat__ = 'restructuredtext en' __docformat__ = 'restructuredtext en'
import re, weakref import re, weakref, operator
from functools import partial from functools import partial
from datetime import timedelta from datetime import timedelta
from collections import deque from collections import deque, OrderedDict
from calibre.constants import preferred_encoding from calibre.constants import preferred_encoding
from calibre.db.utils import force_to_bool from calibre.db.utils import force_to_bool
@ -85,14 +85,14 @@ def _match(query, value, matchkind, use_primary_find_in_search=True):
class DateSearch(object): # {{{ class DateSearch(object): # {{{
def __init__(self): def __init__(self):
self.operators = { self.operators = OrderedDict((
'=': (1, self.eq), ('!=', self.ne),
'!=': (2, self.ne), ('>=', self.ge),
'>': (1, self.gt), ('<=', self.le),
'>=': (2, self.ge), ('=', self.eq),
'<': (1, self.lt), ('>', self.gt),
'<=': (2, self.le), ('<', self.lt),
} ))
self.local_today = {'_today', 'today', icu_lower(_('today'))} self.local_today = {'_today', 'today', icu_lower(_('today'))}
self.local_yesterday = {'_yesterday', 'yesterday', icu_lower(_('yesterday'))} self.local_yesterday = {'_yesterday', 'yesterday', icu_lower(_('yesterday'))}
self.local_thismonth = {'_thismonth', 'thismonth', icu_lower(_('thismonth'))} self.local_thismonth = {'_thismonth', 'thismonth', icu_lower(_('thismonth'))}
@ -158,13 +158,12 @@ class DateSearch(object): # {{{
matches |= book_ids matches |= book_ids
return matches return matches
relop = None for k, relop in self.operators.iteritems():
for k, op in self.operators.iteritems():
if query.startswith(k): if query.startswith(k):
p, relop = op query = query[len(k):]
query = query[p:] break
if relop is None: else:
relop = self.operators['='][-1] relop = self.operators['=']
if query in self.local_today: if query in self.local_today:
qd = now() qd = now()
@ -206,14 +205,14 @@ class DateSearch(object): # {{{
class NumericSearch(object): # {{{ class NumericSearch(object): # {{{
def __init__(self): def __init__(self):
self.operators = { self.operators = OrderedDict((
'=':(1, lambda r, q: r == q), ('!=', operator.ne),
'>':(1, lambda r, q: r is not None and r > q), ('>=', operator.ge),
'<':(1, lambda r, q: r is not None and r < q), ('<=', operator.le),
'!=':(2, lambda r, q: r != q), ('=', operator.eq),
'>=':(2, lambda r, q: r is not None and r >= q), ('>', operator.gt),
'<=':(2, lambda r, q: r is not None and r <= q) ('<', operator.lt),
} ))
def __call__(self, query, field_iter, location, datatype, candidates, is_many=False): def __call__(self, query, field_iter, location, datatype, candidates, is_many=False):
matches = set() matches = set()
@ -245,18 +244,17 @@ class NumericSearch(object): # {{{
else: else:
relop = lambda x,y: x is not None relop = lambda x,y: x is not None
else: else:
relop = None for k, relop in self.operators.iteritems():
for k, op in self.operators.iteritems():
if query.startswith(k): if query.startswith(k):
p, relop = op query = query[len(k):]
query = query[p:] break
if relop is None: else:
p, relop = self.operators['='] relop = self.operators['=']
cast = int cast = int
if dt == 'rating': if dt == 'rating':
cast = lambda x: 0 if x is None else int(x) cast = lambda x: 0 if x is None else int(x)
adjust = lambda x: x/2 adjust = lambda x: x // 2
elif dt in ('float', 'composite'): elif dt in ('float', 'composite'):
cast = float cast = float

View File

@ -305,7 +305,7 @@ class ReadingTest(BaseTest):
old.conn.close() old.conn.close()
old = None old = None
cache = self.init_cache(self.library_path) cache = self.init_cache(self.cloned_library)
for query, ans in oldvals.iteritems(): for query, ans in oldvals.iteritems():
nr = cache.search(query, '') nr = cache.search(query, '')
self.assertEqual(ans, nr, self.assertEqual(ans, nr,
@ -316,6 +316,18 @@ class ReadingTest(BaseTest):
self.assertEqual(cache.search('id:1', ''), {1}) self.assertEqual(cache.search('id:1', ''), {1})
self.assertEqual(cache.search('id:>1', ''), {2, 3}) self.assertEqual(cache.search('id:>1', ''), {2, 3})
# Numeric/rating searches with relops in the old db were incorrect, so
# test them specifically here
cache.set_field('rating', {1:4, 2:2, 3:5})
self.assertEqual(cache.search('rating:>2'), set())
self.assertEqual(cache.search('rating:>=2'), {1, 3})
self.assertEqual(cache.search('rating:<2'), {2})
self.assertEqual(cache.search('rating:<=2'), {1, 2, 3})
self.assertEqual(cache.search('rating:<=2'), {1, 2, 3})
self.assertEqual(cache.search('rating:=2'), {1, 3})
self.assertEqual(cache.search('rating:2'), {1, 3})
self.assertEqual(cache.search('rating:!=2'), {2})
# Note that the old db searched uuid for un-prefixed searches, the new # Note that the old db searched uuid for un-prefixed searches, the new
# db does not, for performance # db does not, for performance