Better distinction between tables and views, and show CREATE VIEW

[betodealmeida]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

In SQL Lab, to distinguish between tables and views we prefix the latter with the string [view], which looks ugly. I changed it to display a small icon representing the type — either a table for actual tables, or an eye for views.

Additionally, we don't present any information on views. I added a link to show the actual CREATE VIEW statement that defines a given view.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

Currently most DB engine specs seem to return views as tables, due to limitations in the SQL Alchemy driver. Both Presto and SQLlite return no views, bundling the views with the actual tables. I had to mock the response from the engines to generate the screenshots.

I'm planning to write a PR for Presto next, allowing views to be properly displayed. This might require changes upstream (to https://github.com/dropbox/PyHive), though it's possible to work around it in Superset only.

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

@mistercrunch @khtruong @etr2460

[mistercrunch]

Some minor comments, otherwise LGTM

[villebro]

This is a nice feature. However, it seems to be unnecessarily specific to Presto, and should be fairly easy to enable for other engines, too, by moving as much of the logic as possible to BaseEngineSpec raising NotImplementedError() if not enabled (the Presto spec is getting fairly heavy lately, and there's probably other stuff that could be made more generic to benefit other engines). Probably ok to keep this in Presto for now, and I can probably work on making this accessible to other engines in a later PR.

[betodealmeida]

This is a nice feature. However, it seems to be unnecessarily specific to Presto, and should be fairly easy to enable for other engines, too, by moving as much of the logic as possible to BaseEngineSpec raising NotImplementedError() if not enabled (the Presto spec is getting fairly heavy lately, and there's probably other stuff that could be made more generic to benefit other engines). Probably ok to keep this in Presto for now, and I can probably work on making this accessible to other engines in a later PR.

I agree. I think we need to start by adding an abstract method to BaseEngineSpec for running queries, and then it should be trivial to extend this to other engines. I'll create a ticket and give it a try when I have some bandwidth.

[betodealmeida]

@mistercrunch is this good to go as soon as tests pass?

[villebro]

LGTM. It hit me, that we should also offer a "get CREATE TABLE statement". I often like to add comments for both table and columns, and those would be convenient to see with the click of a button. So down the line perhaps get_create_view could be refactored to get_create_statement and would return the "CREATE TABLE" or "CREATE VIEW", depending on the type. But let's save that for a later PR.

[codecov-io]

Codecov Report

Merging #8213 into master will decrease coverage by 0.06%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master    #8213      +/-   ##
==========================================
- Coverage   65.79%   65.72%   -0.07%     
==========================================
  Files         480      481       +1     
  Lines       23273    23317      +44     
  Branches     2567     2572       +5     
==========================================
+ Hits        15312    15326      +14     
- Misses       7823     7853      +30     
  Partials      138      138

Impacted Files	Coverage Δ
superset/views/core.py	70.41% <ø> (ø)	⬆️
superset/assets/src/components/TableSelector.jsx	77.69% <0%> (-6.48%)	⬇️
superset/db_engine_specs/base.py	85.61% <40%> (-0.49%)	⬇️
...rset/assets/src/SqlLab/components/TableElement.jsx	83.52% <50%> (-0.81%)	⬇️
superset/db_engine_specs/presto.py	72.37% <53.33%> (-1.48%)	⬇️
superset/assets/src/SqlLab/components/ShowSQL.jsx	55.55% <55.55%> (ø)

---

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 4132d8f...d93c426. Read the comment docs.