[SQLLab] A query with keyword instead of a literal runs into "Only `SELECT` statements are allowed against this database"

[mrshu]

When trying to run the following query

WITH final AS (
  SELECT * 
  FROM b
)
SELECT * FROM final

in Superset's SQL Lab one runs into the "Only SELECT statements are allowed against this database" security exception, which is triggered on this line: https://github.com/apache/incubator-superset/blob/master/superset/sql_lab.py#L156

At first, I though that there was a bug with Superset's dealing with WITH queries, but it turns out that the reason is much more straightforward: since sqlparse considers final to be a keyword (as you can see here), this whole query won't get properly persed, calling get_type() will return UNKNOWN, the "pessimistic readonly" will return False and the security exception will be triggered.

I am not exactly sure what would be the proper way of dealing with a situation like this, as final is not really considered in Presto and the query from above would execute properly and I am pretty sure there are some good reasons why sqlparse considers final a keyword. I am therefore opening this issue in hopes that it may help save some debugging efforts in the future.

That being said, since we know that this happens when the get_type() call returns UNKNOWN, would it make sense to raise a different exception than the standard "Only SELECT statements are allowed against this database"? It may provide the user with more information as to how could they alter the query to around this problem -- currently the only recourse seems to be setting "Allow DML" for the connection.

If you do not find this issue actionable, please feel free to close it -- I am totally OK with that.

Thanks!

---

• I have checked the superset logs for python stacktraces and included it here as text if there are any.
• I have reproduced the issue with at least the latest released version of superset.
• I have checked the issue tracker for the same issue and I haven't found one similar.

Superset version

0.28

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[mrshu]

@kristw @mistercrunch Would you guys be interested in a PR that would raise a different exception when the get_type() function call returns UNKNOWN? I'd be happy to put it together.

Thanks!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[jhua-wish]

This issue still exists in the latest superset. I'm suggesting to bump sqlparse package to the newest instead of 0.3.0

[raags]

I stumbled into the same issue - would suggest we keep this issue open. And an interim solution could be a better error message?

[pacholoamit]

Any updates on this?

[rumbin]

BTW, source named CTEs will also cause this error.

[coolinstarxckr]

I ran into the same issue......any updates on this?

[mistercrunch]

There's a SIP to move from sqlparse to sqlglot here #26786. I'm guessing/hoping it'll help with this. Tagging @betodealmeida who wrote the proposal for visibility.

[rumbin]

Adding sha to the list of evil words, just for documentation

[GuruprasadaShridharHegde]

This is what worked for me:

I was using final in my sql which is a reserved keyword, after that i enclosed with double quotes like "final" which worked for me.. Make sure whenever you refer to the RESERVED keyword then enclose with quotes.. Thanks All !!

  "final" AS (
    SELECT
      vw.*,
      TRIM(gc.first_name||' '||gc.last_name) AS "influencer_name"
    FROM "vw" AS vw