Skip to content

Conversation

@hzhu212
Copy link

@hzhu212 hzhu212 commented Dec 20, 2020

Hi guys, I found a quite obvious bug when I use the Bound filter.

The minimum code to reproduce the bug is as below:

from pydruid.utils.filters import Bound

# I want to select the rows that satisfy "foo >= 0"
Bound(dimension='foo', lower=0).show()

this will cause the following exception:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-76-afbba87ff9ee> in <module>
      1 from pydruid.utils.filters import Bound
----> 2 Bound(dimension='foo', lower=0).show()

~/Library/Python/3.7/lib/python/site-packages/pydruid/utils/filters.py in __init__(self, dimension, lower, upper, lowerStrict, upperStrict, alphaNumeric, ordering, extraction_function)
    216     ):
    217         if not lower and not upper:
--> 218             raise ValueError("Must include either lower or upper or both")
    219         Filter.__init__(
    220             self,

ValueError: Must include either lower or upper or both

But when I changed lower=0 into lower=1, everything just went well:

Bound(dimension='foo', lower=1).show()

output:

{
    "filter": {
        "type": "bound",
        "dimension": "foo",
        "lower": 1,
        "lowerStrict": false,
        "upper": null,
        "upperStrict": false,
        "alphaNumeric": false,
        "ordering": "lexicographic"
    }
}

Obviously, the root of the problem lies in line 217, which is to check whether argument lower and upper are both omitted:

if not lower and not upper:

However, this checking unexpectedly blocked the number 0, besides None.

Changing this line into the line below should fix the bug:

if lower is None and upper is None:

@pranatishete
Copy link

@hzhu212 This property can be overcome if you pass ordering="numeric" and pass lower and upperbound as 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants