varnishcache / varnish-cache

Varnish Cache source code repository
https://www.varnish-cache.org
Other
3.56k stars 365 forks source link

vsl-query: floats do not behave as expected #4088

Closed nigoroll closed 1 month ago

nigoroll commented 3 months ago

Expected Behavior

Comparisons involving floats should behave as documented:

  • == != < <= > >=

Numerical comparison. The record contents will be converted to either an integer or a float before comparison, depending on the type of the operand

Current Behavior

Comparisons fail in unexpected ways

OK:

$ varnishlog -r varnishlog.err-20240314 -q 'Timestamp:Process[2] > 84.5 and Timestamp:Process[2] < 85.' -I Timestamp:Process|head -2                                            
*   << Request  >> 337084421 
-   Timestamp      Process: 1710329152.663188 84.677353 0.000041
$ varnishlog -r varnishlog.err-20240314 -q 'Timestamp:Process[2] > 84.6 and Timestamp:Process[2] < 85.' -I Timestamp:Process|head -2    
*   << Request  >> 337084421 
-   Timestamp      Process: 1710329152.663188 84.677353 0.000041
$ varnishlog -r varnishlog.err-20240314 -q 'Timestamp:Process[2] > 84.67 and Timestamp:Process[2] < 85.' -I Timestamp:Process|head -2
*   << Request  >> 337084421 
-   Timestamp      Process: 1710329152.663188 84.677353 0.000041

not OK:

$ varnishlog -r varnishlog.err-20240314 -q 'Timestamp:Process[2] > 84.677 and Timestamp:Process[2] < 85.' -I Timestamp:Process|head -2
*   << Request  >> 337346562 
-   Timestamp      Process: 1710329152.663282 84.678016 0.000011

Possible Solution

No response

Steps to Reproduce (for bugs)

No response

Context

(gdb) p (double)84.678016
$1 = 84.678016
(gdb) p (float)84.678016
$2 = 84.6780167
(gdb) p (float)84.677
$3 = 84.677002

Varnish Cache version

No response

Operating system

No response

Source of binary packages used (if any)

No response

nigoroll commented 3 months ago

It looks to me like this might be related to use of SF_Parse_Decimal() since 05e10eee3076e4c416ebd460cfa70f4fa8fcdb67.

Questions:

Are structured field semantics a good fit for vsl queries?

Even if they were, is the behavior correct? Do I understand correctly that according to RFC8941, three digits of the fractional component should be significant without any rounding involved? It looks to me like, in the example, 84.677 got rounded to 84.678 ?

Edit: I was wrong in the above. In the example, both Numbers 84.677353 and 84.677 get rounded to 84.677, such that > evaluates to false.

bsdphk commented 3 months ago

In retrospect, SF_Parse_Decimal() is too brutal for delta-T values in VSL records.

nigoroll commented 3 months ago

from 1:1 conversation with phk: we should probably just use strtod() in vsl