[GitHub] [incubator-superset] villebro commented on a change in pull request #11868: chore: Support SET & SHOW commands as read only SQL commands

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

villebro commented on a change in pull request #11868:
URL: https://github.com/apache/incubator-superset/pull/11868#discussion_r534774434



##########
File path: superset/db_engine_specs/hive.py
##########
@@ -525,3 +526,13 @@ def get_function_names(cls, database: "Database") -> List[str]:
         :return: A list of function names useable in the database
         """
         return database.get_df("SHOW FUNCTIONS")["tab_name"].tolist()
+
+    @classmethod
+    def is_readonly(cls, parsed_query: ParsedQuery) -> bool:
+        """Pessimistic readonly, 100% sure statement won't mutate anything"""
+        return (
+            parsed_query.is_select()
+            or parsed_query.is_explain()
+            or parsed_query.is_set()
+            or parsed_query.is_show()

Review comment:
       To avoid duplication of logic, we should probably do `super().is_readonly() or ...` here.

##########
File path: superset/db_engine_specs/base.py
##########
@@ -1038,3 +1038,8 @@ def get_extra_params(database: "Database") -> Dict[str, Any]:
                 logger.error(ex)
                 raise ex
         return extra
+
+    @classmethod
+    def is_readonly(cls, parsed_query: ParsedQuery) -> bool:
+        """Pessimistic readonly, 100% sure statement won't mutate anything"""
+        return parsed_query.is_select() or parsed_query.is_explain()

Review comment:
       As this is moved to a more generic context, it might be a good idea to call this method `is_readonly_query`.

##########
File path: superset/db_engine_specs/presto.py
##########
@@ -1090,3 +1090,12 @@ def extract_errors(cls, ex: Exception) -> List[Dict[str, Any]]:
                 )
             )
         ]
+
+    @classmethod
+    def is_readonly(cls, parsed_query: ParsedQuery) -> bool:
+        """Pessimistic readonly, 100% sure statement won't mutate anything"""
+        return (
+            parsed_query.is_select()
+            or parsed_query.is_explain()
+            or parsed_query.is_show()

Review comment:
       same here




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org