fix(explore): strip semicolons in virtual table SQL

[pkdotson]

SUMMARY

This pr fixes the error when a sql statement in the datasource editor is ended with semicolon. It would cause an error in explore.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

Create a virtual datasource query and and save the dataset. Then click on the datasource to land in explore and the chart panel should not give any error.

ADDITIONAL INFORMATION

• [x ] Has associated issue: [Explore] Chart should strip ending semicolon for virtual datasource queries #12963
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Merging #13801 (bc4718b) into master (541d23b) will decrease coverage by 0.14%.
The diff coverage is 75.00%.

❗ Current head bc4718b differs from pull request most recent head 77a4dcd. Consider uploading reports for the commit 77a4dcd to get more accurate results

@@            Coverage Diff             @@
##           master   #13801      +/-   ##
==========================================
- Coverage   78.35%   78.20%   -0.15%     
==========================================
  Files         934      934              
  Lines       47349    47348       -1     
  Branches     5941     5943       +2     
==========================================
- Hits        37101    37030      -71     
- Misses      10104    10174      +70     
  Partials      144      144              

Flag	Coverage Δ
cypress	56.04% <14.28%> (+<0.01%)	⬆️
hive	?
mysql	80.56% <100.00%> (-0.01%)	⬇️
postgres	80.59% <100.00%> (-0.01%)	⬇️
presto	80.28% <100.00%> (-0.01%)	⬇️
python	80.86% <100.00%> (-0.28%)	⬇️
sqlite	80.16% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...et-frontend/src/SqlLab/components/TableElement.jsx	88.60% <ø> (+0.23%)	⬆️
...rontend/src/SqlLab/components/SqlEditorLeftBar.jsx	56.45% <71.42%> (+1.90%)	⬆️
superset/connectors/sqla/models.py	90.52% <100.00%> (-0.26%)	⬇️
superset/db_engines/hive.py	0.00% <0.00%> (-82.15%)	⬇️
superset/db_engine_specs/hive.py	74.42% <0.00%> (-16.42%)	⬇️
superset/utils/core.py	88.83% <0.00%> (-0.13%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 541d23b...77a4dcd. Read the comment docs.

[etr2460]

one code quality comment

[ktmud]

I don't think you need a new method just for this. The fix should be as simple as this diff:

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 2b61520cd..ea874167b 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -833,6 +833,8 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
         """
         Render sql with template engine (Jinja).
         """
+        if not self.sql:
+            return ""
         sql = self.sql
         if template_processor:
             try:
@@ -844,7 +846,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
                         msg=ex.message,
                     )
                 )
-        sql = sqlparse.format(sql, strip_comments=True)
+        sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True)
         if not sql:
             raise QueryObjectValidationError(_("Virtual dataset query cannot be empty"))
         if len(sqlparse.split(sql)) > 1:

[ktmud]

Can we also add a unit test somewhere here:

superset/tests/sqla_models_tests.py

Line 226 in 609c359

def test_multiple_sql_statements_raises_exception(self):

You can create a new test case with table.sql ending in ; and assert ; is not in the output SQL generated by get_sqla_query.

[villebro]

LGTM

[etr2460]

one comment, but lgtm otherwise. and thanks for adding the test!