Filter empty strings or nulls, and add more operators (#704)

* Filter empty strings or nulls, and add more operators

* Encapsulate strings for translation

caravel/assets/javascripts/explore.js

@@ -294,9 +294,8 @@ function initExploreView() {
   function set_filters() {
     ["flt", "having"].forEach(function (prefix) {
       for (var i = 1; i < 10; i++) {
-        var eq = px.getParam(prefix + "_eq_" + i);
         var col = px.getParam(prefix + "_col_" + i);
-        if (eq !== '' && col !== '') {
+        if (col !== '') {
           add_filter(i, prefix);
         }
       }

caravel/forms.py

@@ -1009,12 +1009,14 @@ def add_to_form(attrs):
             field_css_classes['granularity'] = ['form-control', 'select2_freeform']
             field_css_classes['druid_time_origin'] = ['form-control', 'select2_freeform']
             filter_choices = self.choicify(['in', 'not in', 'regex'])
-            having_op_choices = self.choicify(['>', '<', '=='])
+            having_op_choices = self.choicify(
+                ['==', '!=', '>', '<', '>=', '<='])
             filter_prefixes += ['having']
         add_to_form(('since', 'until'))
 
+        # filter_cols defaults to ''. Filters with blank col will be ignored
         filter_cols = self.choicify(
-            viz.datasource.filterable_column_names or [''])
+            ([''] + viz.datasource.filterable_column_names) or [''])
         having_cols = filter_cols + viz.datasource.metrics_combo
         for field_prefix in filter_prefixes:
             is_having_filter = field_prefix == 'having'

caravel/models.py

@@ -28,7 +28,7 @@
 from pydruid.client import PyDruid
 from pydruid.utils.filters import Dimension, Filter
 from pydruid.utils.postaggregator import Postaggregator
-from pydruid.utils.having import Having, Aggregation
+from pydruid.utils.having import Aggregation
 from six import string_types
 from sqlalchemy import (
     Column, Integer, String, ForeignKey, Text, Boolean, DateTime, Date,
@@ -1283,7 +1283,7 @@ def recursive_get_fields(_conf):
         if filters:
             qry['filter'] = filters
 
-        having_filters = self.get_having_filters(extras.get('having'))
+        having_filters = self.get_having_filters(extras.get('having_druid'))
         if having_filters:
             qry['having'] = having_filters
 
@@ -1399,26 +1399,37 @@ def get_filters(raw_filters):
                 filters = cond
         return filters
 
+    def _get_having_obj(self, col, op, eq):
+        cond = None
+        if op == '==':
+            if col in self.column_names:
+                cond = DimSelector(dimension=col, value=eq)
+            else:
+                cond = Aggregation(col) == eq
+        elif op == '>':
+            cond = Aggregation(col) > eq
+        elif op == '<':
+            cond = Aggregation(col) < eq
+
+        return cond
+
     def get_having_filters(self, raw_filters):
         filters = None
+        reversed_op_map = {
+            '!=': '==',
+            '>=': '<',
+            '<=': '>'
+        }
+
         for col, op, eq in raw_filters:
             cond = None
-            if op == '==':
-                if col in self.column_names:
-                    cond = DimSelector(dimension=col, value=eq)
-                else:
-                    cond = Aggregation(col) == eq
-            elif op == '!=':
-                cond = ~(Aggregation(col) == eq)
-            elif op == '>':
-                cond = Aggregation(col) > eq
-            elif op == '<':
-                cond = Aggregation(col) < eq
+            if op in ['==', '>', '<']:
+                cond = self._get_having_obj(col, op, eq)
+            elif op in reversed_op_map:
+                cond = ~self._get_having_obj(col, reversed_op_map[op], eq)
+
             if filters:
-                filters = Filter(type="and", fields=[
-                    Having.build_having(cond),
-                    Having.build_having(filters)
-                ])
+                filters = filters & cond
             else:
                 filters = cond
         return filters

caravel/templates/caravel/explore.html

@@ -147,7 +147,7 @@
                 <span class="legend_label">{{ _("Filters") }}</span>
                 <i  class="fa fa-info-circle" data-toggle="tooltip"
                     data-placement="bottom"
-                    title="{{_("Filters are defined using comma delimited strings as in 'US,FR,Other'")}}"></i>
+                    title="{{_("Filters are defined using comma delimited strings as in 'US,FR,Other'")}}. {{_("Leave the value field empty to filter empty strings or nulls")}}"></i>
                 <span class="collapser"> [-]</span>
               </div>
               <div class="panel-body">
@@ -177,7 +177,7 @@
                   <span class="legend_label">Result Filters ("having" filters)</span>
                   <i class="fa fa-info-circle" data-toggle="tooltip"
                      data-placement="bottom"
-                     title="The filters to apply after post-aggregation"></i>
+                     title="{{_("The filters to apply after post-aggregation.")}} {{_("Leave the value field empty to filter empty strings or nulls")}}"></i>
                   <span class="collapser"> [-]</span>
                 </div>
                 <div class="panel-body">

caravel/viz.py

@@ -198,7 +198,7 @@ def query_filters(self, is_having_filter=False):
             col = form_data.get(field_prefix + "_col_" + str(i))
             op = form_data.get(field_prefix + "_op_" + str(i))
             eq = form_data.get(field_prefix + "_eq_" + str(i))
-            if col and op and eq:
+            if col and op and eq is not None:
                 filters.append((col, op, eq))
 
         # Extra filters (coming from dashboard)
@@ -236,7 +236,8 @@ def query_obj(self):
         # for instance the extra where clause that applies only to Tables
         extras = {
             'where': form_data.get("where", ''),
-            'having': self.query_filters(True) or form_data.get("having", ''),
+            'having': form_data.get("having", ''),
+            'having_druid': self.query_filters(True),
             'time_grain_sqla': form_data.get("time_grain_sqla", ''),
             'druid_time_origin': form_data.get("druid_time_origin", ''),
         }