[GitHub] GeorgeSirois commented on a change in pull request #4016: [cache] Using the query as the basis of the cache key

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

GeorgeSirois commented on a change in pull request #4016: [cache] Using the query as the basis of the cache key
URL: https://github.com/apache/incubator-superset/pull/4016#discussion_r159778054
 
 

 ##########
 File path: superset/viz.py
 ##########
 @@ -225,44 +224,50 @@ def get_json(self, force=False):
             self.get_payload(force),
             default=utils.json_int_dttm_ser, ignore_nan=True)
 
-    @property
-    def cache_key(self):
-        form_data = self.form_data.copy()
-        merge_extra_filters(form_data)
-        s = str([(k, form_data[k]) for k in sorted(form_data.keys())])
-        return hashlib.md5(s.encode('utf-8')).hexdigest()
+    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.
+        """
+
+        return hashlib.md5(
+            json.dumps((
+                self.datasource.id,
+                self.datasource.get_query_str(query_obj).encode('utf-8'),
 
 Review comment:
   I think this is probably already an issue with the existing setup, but you may also want to consider cases where different users may see different results for the _same query and datasource_ based on out of band information, such as a user that gets passed through in the query context (e.g. [row-level permission in a view](https://cloud.google.com/bigquery/docs/views#row-level-permissions)).

----------------------------------------------------------------
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