SQL with inline comment starting with -- fails in dashboards view query and sqllab

[PedroMartinSteenstrup]

Bug description

Using sql_mutator to append a comment to queries such as -- Superset query context '{"user": "superset.admin" }' (dash dash followed by the object to parse later)

• the view query menu item in dashboards will render Expecting value: line 1 column 1 (char 0)
• the sqllab will error with Statement 2 out of 2 Only SELECT statements are allowed against this database.

Initially I've had MUTATE_AFTER_SPLIT = False.

I've been using this "tagging" of queries for years, so I can definitely link it to something recent, and given the history of commits, I'm not sure what I should do: wait for a fix? or find another way to tag queries?

#29248
#28567
#28363
#27725

How to reproduce the bug

1. Alter sql_mutator following simplified version below

def sql_mutator(sql, security_manager, database):
[...]
    from superset.utils.core import QuerySource, get_username
    user_name = get_username()
[...]
     q_suffix = f'Superset query context'
[...]
     q_suffix = f'{q_suffix} \'{{"{{"}}"user": "{user_name}" {{"}}"}}\''
    return f"{sql} -- {q_suffix}"

2. Run following Sqllab query

SELECT 1;
--select 2;

3. Or, go to a dashboard and try to View Query one chart

4. See results in screenshots

5. If I enable MUTATE_AFTER_SPLIT = True, then the dashboards can load and the View Query functions. But running a sql query in SQLlab where one statement is commented out still does not function if the statement ends with ; (semicolon).

Screenshots/recordings

Superset version

4.0.2 (apache/superset:4.0.2)

Python version

3.9

Node version

I don't know

Browser

Firefox 115.11.0esr
Gecko v0.34.0

Additional context

I am running 4.0.2 from published docker image, as I thought this would bring a fix from the change in behavior introduced when we upgraded to 4.0.1rc.

I could repeat the same error with Snowflake and Postgres databases.

2024-07-03 10:22:48,376:ERROR:superset.views.base:Expecting value: line 1 column 1 (char 0)
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1823, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
  File "/usr/local/lib/python3.9/site-packages/flask_appbuilder/security/decorators.py", line 95, in wraps
    return f(self, *args, **kwargs)
  File "/app/superset/views/base_api.py", line 127, in wraps
    raise ex
  File "/app/superset/views/base_api.py", line 121, in wraps
    duration, response = time_function(f, self, *args, **kwargs)
  File "/app/superset/utils/core.py", line 1470, in time_function
    response = func(*args, **kwargs)
  File "/app/superset/utils/log.py", line 255, in wrapper
    value = f(*args, **kwargs)
  File "/app/superset/charts/data/api.py", line 262, in data
    return self._get_data_response(
  File "/app/superset/charts/data/api.py", line 418, in _get_data_response
    result = command.run(force_cached=force_cached)
  File "/app/superset/commands/chart/data/get_data_command.py", line 45, in run
    payload = self._query_context.get_payload(
  File "/app/superset/common/query_context.py", line 100, in get_payload
    return self._processor.get_payload(cache_query_context, force_cached)
  File "/app/superset/common/query_context_processor.py", line 647, in get_payload
    query_results = [
  File "/app/superset/common/query_context_processor.py", line 648, in <listcomp>
    get_query_results(
  File "/app/superset/common/query_actions.py", line 227, in get_query_results
    return result_func(query_context, query_obj, force_cached)
  File "/app/superset/common/query_actions.py", line 90, in _get_query
    result["query"] = datasource.get_query_str(query_obj.to_dict())
  File "/app/superset/connectors/sqla/models.py", line 1437, in get_query_str
    query_str_ext = self.get_query_str_extended(query_obj)
  File "/app/superset/connectors/sqla/models.py", line 1426, in get_query_str_extended
    sql = self.mutate_query_from_config(sql)
  File "/app/superset/connectors/sqla/models.py", line 1406, in mutate_query_from_config
    sql = sql_query_mutator(
  File "/app/pythonpath/superset_config.py", line 647, in sql_mutator
    dash_id = json.loads(request.args.get('dashboard_id', 'NULL'))
  File "/usr/local/lib/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/local/lib/python3.9/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/local/lib/python3.9/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Checklist

• I have searched Superset docs and Slack and didn't find a solution to my problem.
• I have searched the GitHub issue tracker and didn't find a similar bug report.
• I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

[dosubot]

@PedroMartinSteenstrup Hello there! I'm here to help you with any bugs, questions, or contributions you have in mind. Let's squash those bugs together! 😊

The issue you're encountering with SQL queries containing inline comments has been addressed in several pull requests, including #27725, #28363, #28567, and #29248. These changes ensure that comments are not stripped when executing SQL statements, which should resolve the errors you're seeing in both the dashboards' "view query" and SQL Lab.

To fix the issue, ensure you are using the latest version of Superset where these fixes have been merged. Specifically, the changes in pull request #29248 should address the problem you're facing with inline comments starting with -- ^[1].

If you are already on the latest version and still experiencing issues, double-check your sql_mutator function implementation and ensure it aligns with the updated handling of SQL comments. Here is a simplified example of how you might define the sql_mutator function:

def sql_mutator(sql, security_manager, database):
    from superset.utils.core import get_username
    user_name = get_username()
    q_suffix = f'Superset query context \'{{"user": "{user_name}"}}\''
    return f"{sql} -- {q_suffix}"

Make sure your Superset configuration includes this function and that MUTATE_AFTER_SPLIT is set according to your needs:

SQL_QUERY_MUTATOR = sql_mutator
MUTATE_AFTER_SPLIT = False

This should help resolve the errors related to inline comments in your SQL queries.

To continue talking to Dosu, mention @dosu.

[PedroMartinSteenstrup]

good bot @dosu, setting MUTATE_AFTER_SPLIT = True helped with the dashboards at least.

Sadly, still a problem in sqllab, and I can now even more precisely point it out:

That will work:

select * from reporting.dim_date limit 100
-- select * from information_schema.functions limit 10;

That will not work:

select * from reporting.dim_date limit 100;
-- select * from information_schema.functions limit 10;

The difference? Statement ending with semicolon.

[dosubot]

The issue with SQL Lab in Apache Superset when a SQL statement containing inline comments ends with a semicolon can be resolved by ensuring that the SQL statement is properly parsed and executed. The ParsedQuery class's get_statements method is responsible for splitting SQL queries into individual statements. It ensures that each statement is cleaned and separated properly before being added to the list of statements.

Here is the specific logic used in the get_statements method:

def get_statements(self) -> list[str]:
    """Returns a list of SQL statements as strings, stripped"""
    statements = []
    for statement in self._parsed:
        if statement:
            sql = str(statement).strip(" \n;\t")
            if sql:
                statements.append(sql)
    return statements

This method strips any leading or trailing whitespace, newline characters, semicolons, and tabs from each statement, ensuring they are correctly parsed ^[1].

Additionally, there are known issues with the sqlparse library used by Superset for parsing SQL queries, particularly with handling inline comments and semicolons. A workaround has been implemented in Superset's code to address these issues ^[1].

To further troubleshoot and resolve the issue, you can:

1. Verify the ParsedQuery Class: Ensure that the ParsedQuery class and its get_statements method correctly parse the SQL statements, especially handling inline comments and semicolons.

2. Check for Known Issues: Be aware of the known issues with the sqlparse library and ensure that any necessary workarounds are applied.

3. Update Superset: Make sure you are using the latest version of Superset where fixes for handling comments and semicolons have been merged.

By following these steps, you should be able to resolve the issue with SQL Lab when a SQL statement containing inline comments ends with a semicolon.

To continue talking to Dosu, mention @dosu.