[GitHub] mistercrunch closed pull request #4260: Use the query_obj as the basis for the cache key

[GitBox <gi...@apache.org>]

mistercrunch closed pull request #4260: Use the query_obj as the basis for the cache key
URL: https://github.com/apache/incubator-superset/pull/4260
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py
index 57038097b7..64641e8ed1 100644
--- a/superset/connectors/druid/models.py
+++ b/superset/connectors/druid/models.py
@@ -1037,7 +1037,7 @@ def run_query(  # noqa / druid
             inner_from_dttm=None, inner_to_dttm=None,
             orderby=None,
             extras=None,  # noqa
-            columns=None, phase=2, client=None, form_data=None,
+            columns=None, phase=2, client=None,
             order_desc=True,
             prequeries=None,
             is_prequery=False,
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 6ccddbe79c..b2e21c7d0f 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -446,7 +446,6 @@ def get_sqla_query(  # sqla
             orderby=None,
             extras=None,
             columns=None,
-            form_data=None,
             order_desc=True,
             prequeries=None,
             is_prequery=False,
@@ -458,7 +457,6 @@ def get_sqla_query(  # sqla
             'metrics': metrics,
             'row_limit': row_limit,
             'to_dttm': to_dttm,
-            'form_data': form_data,
         }
         template_processor = self.get_template_processor(**template_kwargs)
         db_engine_spec = self.database.db_engine_spec
@@ -654,7 +652,6 @@ def get_sqla_query(  # sqla
                     'orderby': orderby,
                     'extras': extras,
                     'columns': columns,
-                    'form_data': form_data,
                     'order_desc': True,
                 }
                 result = self.query(subquery_obj)
diff --git a/superset/viz.py b/superset/viz.py
index bb0bcf604d..91893f85f8 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -205,7 +205,6 @@ def query_obj(self):
             'timeseries_limit': limit,
             'extras': extras,
             'timeseries_limit_metric': timeseries_limit_metric,
-            'form_data': form_data,
             'order_desc': order_desc,
             'prequeries': [],
             'is_prequery': False,
@@ -229,16 +228,22 @@ def get_json(self, force=False):
 
     def cache_key(self, query_obj):
         """
-        The cache key is the datasource/query string tuple associated with the
-        object which needs to be fully deterministic.
+        The cache key is made out of the key/values in `query_obj`
+
+        We remove datetime bounds that are hard values,
+        and replace them with the use-provided inputs to bounds, which
+        may we time-relative (as in "5 days ago" or "now").
         """
+        cache_dict = copy.deepcopy(query_obj)
+
+        for k in ['from_dttm', 'to_dttm']:
+            del cache_dict[k]
+
+        for k in ['since', 'until', 'datasource']:
+            cache_dict[k] = self.form_data.get(k)
 
-        return hashlib.md5(
-            json.dumps((
-                self.datasource.id,
-                self.datasource.get_query_str(query_obj),
-            )).encode('utf-8'),
-        ).hexdigest()
+        json_data = self.json_dumps(cache_dict, sort_keys=True)
+        return hashlib.md5(json_data.encode('utf-8')).hexdigest()
 
     def get_payload(self, force=False):
         """Handles caching around the json payload retrieval"""
@@ -320,8 +325,13 @@ def get_payload(self, force=False):
             'rowcount': rowcount,
         }
 
-    def json_dumps(self, obj):
-        return json.dumps(obj, default=utils.json_int_dttm_ser, ignore_nan=True)
+    def json_dumps(self, obj, sort_keys=False):
+        return json.dumps(
+            obj,
+            default=utils.json_int_dttm_ser,
+            ignore_nan=True,
+            sort_keys=sort_keys,
+        )
 
     @property
     def data(self):
@@ -431,9 +441,10 @@ def get_data(self, df):
             columns=list(df.columns),
         )
 
-    def json_dumps(self, obj):
+    def json_dumps(self, obj, sort_keys=False):
         if self.form_data.get('all_columns'):
-            return json.dumps(obj, default=utils.json_iso_dttm_ser)
+            return json.dumps(
+                obj, default=utils.json_iso_dttm_ser, sort_keys=sort_keys)
         else:
             return super(TableViz, self).json_dumps(obj)
 
diff --git a/tests/core_tests.py b/tests/core_tests.py
index a7edc4ec16..367ab68950 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -83,6 +83,18 @@ def test_slice_endpoint(self):
             '/superset/slice/{}/?standalone=true'.format(slc.id))
         assert 'List Roles' not in resp
 
+    def test_cache_key(self):
+        self.login(username='admin')
+        slc = self.get_slice('Girls', db.session)
+
+        viz = slc.viz
+        qobj = viz.query_obj()
+        cache_key = viz.cache_key(qobj)
+        self.assertEqual(cache_key, viz.cache_key(qobj))
+
+        qobj['groupby'] = []
+        self.assertNotEqual(cache_key, viz.cache_key(qobj))
+
     def test_slice_json_endpoint(self):
         self.login(username='admin')
         slc = self.get_slice('Girls', db.session)


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services