[GitHub] [superset] john-bodley commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator

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

john-bodley commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564992134



##########
File path: superset/viz.py
##########
@@ -474,17 +480,30 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
         filters = self.form_data.get("filters", [])
         filter_columns = [flt.get("col") for flt in filters]
         columns = set(self.datasource.column_names)
+        filter_values_columns = []
+
+        # if using virtual datasource, check filter_values
+        if self.datasource.sql:
+            statements_without_comments = sqlparse.format(

Review comment:
       Because we're doing regex of the SQL statement (and not actual parsing of the statement) this seems somewhat of a hybrid approach. I would likely either:
   
   1. Don't worry about parsing the statement and just run the regex logic on the full SQL statement.
   2. Leverage `sqlparse` to actually identify the `filter_value` function and the items in the identifier list, i.e., 
   
   ```
   stmt._pprint_tree()
   ...
         |     |- 7 Function 'filter...'
         |     |  |- 0 Identifier 'filter...'
         |     |  |  `- 0 Name 'filter...'
         |     |  `- 1 Parenthesis '('my_d...'
         |     |     |- 0 Punctuation '('
         |     |     |- 1 IdentifierList ''my_da...'
         |     |     |  |- 0 Single ''my_da...'
         |     |     |  |- 1 Punctuation ','
         |     |     |  |- 2 Whitespace ' '
         |     |     |  `- 3 Single ''birth...'
         |     |     `- 2 Punctuation ')'
   ...
   ```




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