fix: PrestoEngineSpec._show_columns return type

[john-bodley]

SUMMARY

The PrestoEngineSpec._show_columns method stated it returned a List[RowProxy] type however it actually returned a ResultProxy, i.e., per the SQLAlchemy documentation,

The RowProxy object is retrieved from a database result, from the ResultProxy object using methods like ResultProxy.fetchall().

From a coding perspective this wasn't problematic as the output is later processed via,

for column in PrestoEngineSpec._show_columns(...):
 ....

and though the ResultProxy supports iteration (per the DB API), it does not support pickling (and thus caching) whereas the RowProxy does (it's acts somewhat akin to a Python tuple). This was resulting in an issue at Airbnb where we actually override and cache the PrestoEngineSpec._show_columns using a user lock to help reduce the number of queries we send to Presto.

The fix is merely to augment the method to return a List[RowProxy] as promised rather than a ResultProxy.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

CI.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Merging #20757 (48408ec) into master (e60083b) will decrease coverage by 11.48%.
The diff coverage is 100.00%.

❗ Current head 48408ec differs from pull request most recent head 6a611f9. Consider uploading reports for the commit 6a611f9 to get more accurate results

@@             Coverage Diff             @@
##           master   #20757       +/-   ##
===========================================
- Coverage   66.35%   54.87%   -11.49%     
===========================================
  Files        1754     1754               
  Lines       66689    66688        -1     
  Branches     7049     7049               
===========================================
- Hits        44253    36594     -7659     
- Misses      20639    28297     +7658     
  Partials     1797     1797               

Flag	Coverage Δ
hive	53.23% <0.00%> (+<0.01%)	⬆️
mysql	?
postgres	?
presto	53.09% <100.00%> (-0.01%)	⬇️
python	58.00% <100.00%> (-23.69%)	⬇️
sqlite	?
unit	50.57% <0.00%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
superset/db_engine_specs/presto.py	39.54% <100.00%> (-49.51%)	⬇️
superset/utils/dashboard_import_export.py	0.00% <0.00%> (-100.00%)	⬇️
superset/key_value/commands/update.py	0.00% <0.00%> (-88.89%)	⬇️
superset/key_value/commands/delete.py	0.00% <0.00%> (-85.30%)	⬇️
superset/key_value/commands/delete_expired.py	0.00% <0.00%> (-80.77%)	⬇️
superset/dashboards/commands/importers/v0.py	15.62% <0.00%> (-76.25%)	⬇️
superset/datasets/commands/update.py	25.30% <0.00%> (-68.68%)	⬇️
superset/datasets/commands/create.py	29.41% <0.00%> (-68.63%)	⬇️
superset/datasets/commands/importers/v0.py	24.03% <0.00%> (-67.45%)	⬇️
superset/reports/commands/execute.py	24.45% <0.00%> (-67.16%)	⬇️
... and 275 more

---

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 e60083b...6a611f9. Read the comment docs.