search

druid-io / pydruid

A Python connector for Druid
Other
509 stars 200 forks source link

Updated bound filter to latest Druid API specs #170

Closed wjdecorte closed 5 years ago

wjdecorte commented 5 years ago

Updated the Bound class and Filter class with the correct parameter name for latest version of Druid. Changed "alphanumeric" to "ordering" and set the default to lexicographic based on API documentation. Updated the Bound Filter tests with new parameter and executed all tests successfully.

mistercrunch commented 5 years ago

Wait what about people using older versions of Druid? Is there a way to support both params? Is there a related Druid issue or mailing list threasd that describe the backward incompatible change?

wjdecorte commented 5 years ago

It was changed in version 0.9.2 of Druid. Druid Issue 3270

I thought about that as well, but decided that the latest version of PyDruid should support the latest version of the Druid. I don't see an easy way to support all versions of Druid. I think it would require checking the version of Druid, which means an additional call to Druid before issuing the query. This would be a performance hit. We can address the backwards incompatibility in the documentation, warning users of older versions of Druid. We could allow including either parameter in the filter and then rely on Druid to either accept or fail the query, though I'm not sure this will be a good reflection on PyDruid (users could blame PyDruid instead of understanding what the real issue is). I'm open to any suggestions on the best course of action. As it stands, any user of Druid from 0.9.2 to the latest cannot currently use Bound Filters with any other sorting than the default lexicographic.

mistercrunch commented 5 years ago

@gianm, thoughts?

wjdecorte commented 5 years ago

@mistercrunch My apologies, I was wrong. I tested using the old parameter "alphaNumeric" with Druid 0.14.0 and the query was successful. I'm going to update my branch to handle both the old and the new parameters.