massucattoj commented on code in PR #41996:
URL: https://github.com/apache/superset/pull/41996#discussion_r3570229749


##########
superset/utils/core.py:
##########
@@ -1456,7 +1458,9 @@ def split_adhoc_filters_into_base_filters(  # pylint: 
disable=invalid-name
                     )
             elif expression_type == "SQL":
                 sql_expression = adhoc_filter.get("sqlExpression")
-                sql_expression = sanitize_clause(sql_expression, engine)
+                # keep a trailing line comment from swallowing the " AND " join
+                if sql_expression and "--" in sql_expression:
+                    sql_expression = f"{sql_expression}\n"
                 if clause == "WHERE":

Review Comment:
   This doesn't produce a silent bypass or malformed SQL reaching the database. 
The comment always swallows its own clause's closing paren, so the joined 
predicate is unbalanced and the downstream dialect-aware validator 
(`_process_sql_expression` -> `sanitize_clause`, which runs after Jinja 
rendering and is the "parser-based sanitizer" this suggestion refers to) 
rejects it with a clear `Invalid SQL clause` error. Verified: 
`sanitize_clause("(a = 1 # comment) AND (b = 2)", "mysql")` raises 
`QueryClauseValidationException`.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to