#155 closed defect (released)
ValueRangeProcessors are undocumented
Reported by: | Richard Boulton | Owned by: | Olly Betts |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | QueryParser | Version: | SVN trunk |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #154 | Operating System: | All |
Description
I can't find any documentation for the new ValueRangeProcessors - the header files have no documentation comments, and there doesn't appear to be anything in docs/.
In particular, it would be useful to know what kind of input the pre-defined value range processors are meant to accept, and how they are intended to be used (ie, what indexing strategy to use with them).
For what it's worth, I think I've worked out from reading the code what the DateValueRangeProcessor is meant to accept, but I can't see how the NumberValueRangeProcessor actually works correctly unless I'm missing something about what it's meant to do.
Change History (6)
comment:1 by , 17 years ago
Blocking: | 154 added |
---|
comment:2 by , 17 years ago
Status: | new → assigned |
---|
I definitely wrote some documentation but it looks like I failed to check it in.
Sorry, I'll track it down.
NumberValueRangeProcessor has feature tests which pass, so it can't be totally broken...
comment:3 by , 17 years ago
My concern with NumberValueRangeProcessor is that it won't work correctly unless all numbers supplied to it are the same length (and are the same length as the numbers stored in the values), because it doesn't do anything to transform the numbers into strings which sort in the correct way.
I've just added a test case (test_qp_value_range3) to queryparsertest.cc which illustrates this - it adds a load of numerical values, and checks that a NumberValueRangeProcessor restricts the results appropriately. It currently fails because "10" is considered to be between "1" and "2".
I think the appropriate fix is to define a function which serialises numbers in a sort-preserving manner (possibly including negative numbers), and to document that values must be stored using that if they're to work with NumberValueRangeProcessor (and obviously, to use that in NumberValueRangeProcessor, too). Of course, this changes the semantics of public methods in 1.0.0, but since they weren't documented this shouldn't be too much of a problem (but might be a justification for a 1.0.1 release sooner rather than later).
comment:4 by , 17 years ago
I've just committed documentation comments for VRP and subclasses, but I'd like to add an overview document as well.
Suggesting this for 1.0.1, since fixing it shouldn't risk breaking anything, and the feature is close to unusable for third-parties without documentation.