bugfix: Improve support for special characters in schema and table names

[villebro]

CATEGORY

• Bug Fix
• Add unit tests
• Refactor

SUMMARY

Schema and table names that contain special characters currently cause the following problems:

• SQL Lab: If schema contains slashes, browsing tables doesn't work. This is due to the schema being passed unquoted in the path of the AJAX call, causing it to add a new component to the path.
• SQL Lab: if table contains slashes, preview of the table doesn't always work for similar reasons as the schema. This has been worked around by using the path:-prefix for table name; however, path: can't be used if there are multiple components in the path containing slashes.
• Engine specs: Snowflake, Presto, Hive and MySQL currently pass the selected schema unquoted to the connection URI, most likely causing all queries to fail if schema contains slashes.

This PR adds quoting to schema and table names that are passed in the URI path, and ensures that unquoting is performed in the receiving end. Quoting is done by calling encodeURIComponent() first on the schema/table name, and finally calling encodeURI() on the full URI. Both calls are required, as only encoding the table/schema component will be converted to a regular slash by most (all?) webservers. In addition, a typo was fixed in the TableSelector onChange() event, and the utils function js_string_to_python was refactored to be more descriptive and generic.

BEFORE

AFTER

TEST PLAN

Tested locally to work both with and without slashes in table and schema names. Has been confirmed to work on Presto with Pulsar connector (slashes in schema name), Postgres (slashes in both schema and table names) and Snowflake (no slashes, mainly for regression testing).

ADDITIONAL INFORMATION

• Has associated issue: Fixes Error in SQL editor with slashes in schema name #7236

REVIEWERS

@yciabaud - Tested on Presto + Pulsar.

[codecov-io]

Codecov Report

Merging #7297 into master will increase coverage by <.01%.
The diff coverage is 80.95%.

@@            Coverage Diff             @@
##           master    #7297      +/-   ##
==========================================
+ Coverage   65.22%   65.23%   +<.01%     
==========================================
  Files         431      431              
  Lines       21147    21154       +7     
  Branches     2343     2343              
==========================================
+ Hits        13794    13800       +6     
- Misses       7237     7238       +1     
  Partials      116      116

Impacted Files	Coverage Δ
superset/assets/src/SqlLab/actions/sqlLab.js	70.64% <0%> (ø)	⬆️
superset/utils/core.py	88.15% <100%> (+0.04%)	⬆️
superset/assets/src/components/TableSelector.jsx	84.67% <100%> (ø)	⬆️
superset/db_engine_specs.py	61.47% <80%> (+0.01%)	⬆️
superset/views/core.py	72.58% <80%> (+0.03%)	⬆️

---

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 5cf454b...65b1e78. Read the comment docs.

[yciabaud]

Hello I tried your branch using docker but I am still having some issues when selecting a table.
There is a call on http://localhost:8088/superset/table/2/generator_test/public%252Fdefault/ with a status 500.
Maybe I did something wrong?

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 2309, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 2295, in wsgi_app
    response = self.handle_exception(e)
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 1741, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.6/site-packages/flask/_compat.py", line 35, in reraise
    raise value
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 2292, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 1815, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 1718, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.6/site-packages/flask/_compat.py", line 35, in reraise
    raise value
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 1813, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/usr/local/lib/python3.6/site-packages/flask_appbuilder/security/decorators.py", line 26, in wraps
    return f(self, *args, **kwargs)
  File "/home/superset/superset/models/core.py", line 1163, in wrapper
    value = f(*args, **kwargs)
  File "/home/superset/superset/views/core.py", line 2404, in table
    cols=columns, latest_partition=True),
  File "/home/superset/superset/models/core.py", line 908, in select_star
    schema=schema, source=utils.sources.get('sql_lab', None))
  File "/home/superset/superset/utils/core.py", line 114, in __call__
    value = self.func(*args, **kwargs)
  File "/home/superset/superset/models/core.py", line 828, in get_sqla_engine
    return create_engine(url, **params)
  File "/usr/local/lib/python3.6/site-packages/sqlalchemy/engine/__init__.py", line 423, in create_engine
    return strategy.create(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/sqlalchemy/engine/strategies.py", line 98, in create
    (cargs, cparams) = dialect.create_connect_args(u)
  File "/usr/local/lib/python3.6/site-packages/pyhive/sqlalchemy_presto.py", line 106, in create_connect_args
    raise ValueError("Unexpected database format {}".format(url.database))
ValueError: Unexpected database format pulsar/public/default

Here the database is "pulsar" and the schema is "public/default". A working query is select * from pulsar."public/default".generator_test;

[yciabaud]

It looks like pyhive is not managing slashes in the DB URL: https://github.com/dropbox/PyHive/blob/master/pyhive/sqlalchemy_presto.py#L92

Will file an issue there too.

[villebro]

It seems schemas can be specified both in the SQLAlchemy URI with a slash after the database (db/schema) name or as a query parameter (?schema=schema), and Superset is currently using the former, while your case would require the latter. That, or just escaping the schema name. Should be easy to fix as long as we know exactly how to construct the URI.

[villebro]

@yciabaud I added a quick fix that might work; try it out and post the stacktrace if not.

[yciabaud]

I am testing this, meanwhile I can confirm I managed to make engine = create_engine('presto://localhost:8081/pulsar?schema=public/default') working.

I guess your fix will do.

[yciabaud]

I confirm everything is working for me now.
Thank you very much for your work.

[villebro]

Ready for review.

[villebro]

@mistercrunch could you take a look at this PR? I added unit tests (both python and js) and fixed existing async tests that weren't functioning properly. This seems to be working well and shouldn't have adverse side-effects.

[mistercrunch]