NumberValueRangeProcessors aren't useful if numbers aren't all fixed width

[Richard Boulton]

Copied from bug #155:

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

[Richard Boulton]

I have a Python implementation of a function to implement appropriate serialisation of numbers (ie, to a form which sorts in numeric order), and am working on a C++ implementation of this, so taking this bug over.

Also, marking as a blocker for 1.0.1, since NumberValueRangeProcessors are nearly unusable there.

[Richard Boulton]

Updated implementation of fix

[Richard Boulton]

I applied this by mistake (strange but true), but Olly was going to review it, so I'm leaving this open for the moment so that doesn't get forgotten. Since it blocks 1.0.2, it shouldn't be missed, and Olly can mark it as resolved once he's reviewed it and is happy with it.

[Olly Betts]

Makes sense - reassigning to myself.

[Mark Hammond]

FWIW, a debug MSVC build is now failing tests.

% queryparsertest.exe value_range_serialise1 Running test: value_range_serialise1... Number: -4.49423e+307 String: w═ Number: -1024.5 String: {┴ ▀ Number: -3.14159 String: {╩mαJ╗╜.⌂ Number: -2 String: {╩ Number: -1.8 String: {╦333333/ Number: -1.1 String: {╦µfffff_ Number: -1 String: {╦ Number: -0.5 String: {╠ Number: -0.2 String: {╬ffffff_ Number: -0.1 String: {╧ffffff_ Number: -5e-006 String: {▌░tºq╔p∩ Number: -2e-006 String: {▐≤Éà⌠í'/ Number: -1e-006 String: {▀≤Éà⌠í'/ Number: -1.11254e-308 String: ⌂╩ Number: 0 String: Ç Number: 0 String: Ç

FAILED

queryparsertest.exe completed test run: 0 tests passed, 1 failed.

[Mark Hammond]

Running with -v shows:

.\queryparsertest.cc:1202: prevnum < num Expected previous number (0) to be less than current number (0)

[Olly Betts]

I've fixed the (testsuite) bug which was causing the failure in comment #6.

[Olly Betts]

Fixed in SVN.

[Richard Boulton]

Fix is in upcoming 1.0.2 release. Marking as closed.