[sql lab] option to disable cross schema search

[mistercrunch]

This is killing our metastore as people type it emits large
all-table-dump as they hit the keystroke. It never returns as it times
out and hammers the poor metastore.

Also some improvements around the disabling the table select on the left
panel and having the table name not be sticky.

[codecov-io]

Codecov Report

Merging #4551 into master will decrease coverage by 0.01%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #4551      +/-   ##
==========================================
- Coverage   71.17%   71.16%   -0.02%     
==========================================
  Files         187      187              
  Lines       14809    14824      +15     
  Branches     1085     1086       +1     
==========================================
+ Hits        10540    10549       +9     
- Misses       4266     4272       +6     
  Partials        3        3

Impacted Files	Coverage Δ
superset/views/core.py	71.2% <ø> (ø)	⬆️
...assets/javascripts/SqlLab/components/SqlEditor.jsx	74.46% <ø> (ø)	⬆️
superset/models/core.py	86.7% <100%> (-0.62%)	⬇️
...javascripts/SqlLab/components/SqlEditorLeftBar.jsx	92.45% <83.33%> (-0.55%)	⬇️
superset/sql_lab.py	74.21% <0%> (-0.13%)	⬇️
superset/config.py	92.18% <0%> (+0.06%)	⬆️

---

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 c85eea3...e78d788. Read the comment docs.

[hughhhh]

nit: list of all tables

[hughhhh]

🚢

[john-bodley]

@mistercrunch it seems after the Alembic upgrade the allow_multi_schema_metadata_fetch is NULL rather than True (which ensures backwards compatibility). I think you need to include logic of the form,

from sqlalchemy.sql import table, column

dbs = table('dbs', column('allow_multi_schema_metadata_fetch'))       
op.execute(dbs.update().values(allow_multi_schema_metadata_fetch=True))  

[mistercrunch]

@john-bodley do you think I should write a db migration script to handle this?

[john-bodley]

Because it’s a fairly recent migration how do you feel about augmenting the existing migration? Alternatively you could make the default in the code False (thus making it a breaking change) which evaluates the same as None (NULL).

I think for us, having this disabled is preferred.

[john-bodley]

@mistercrunch my other concern is if one sets allow_multi_schema_metadata_fetch to False or None then in essence you're disabling the caching logic provided by the update_datasources_cache command which caches the tables associated with every schema per here.

[john-bodley]

Sorry @mistercrunch I'm mistaken, the update_datasource_cache and the table caching logic is only used if allow_multi_schema_metadata_fetch is True, it calls the /tables API endpoint with schema as NULL.

[mistercrunch]

If caching is off it's pretty brutal on the poor database as it hits the endpoint multiple times as users type. A confused user trying to get this to work could results in dozens of stuck calls that scan the metadata tables. On large databases it's not great.

Given that, False is a better default here, I was just trying to avoid breaking changes.

Also if you haven't run that migration, I suggest you schedule downtime for this one. It locked up on our side...

[john-bodley]

@mistercrunch this migration didn't cause a lock on our end. I wonder whether there's also merit in caching when someone chooses a schema. I think there's plan for us to analyze the Presto logs to see the frequency of SHOW SCHEMAS and SHOW TABLES queries and whether there is reason to alleviate additional load on the database.