fix: status 500 thrown when creating a dataset due to invalid sql

[jzhao62]

SUMMARY

previously when a dataset is being created, supersetGenericDB error can be thrown at the db_engine layer if a sql statement is invalid

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

before

https://github.com/apache/superset/assets/25937657/7ee03388-6928-4edd-9770-5acf9b4afb6d

after

https://github.com/apache/superset/assets/25937657/57e6b335-f5cc-4324-9fe1-0fd963aabb9d

TESTING INSTRUCTIONS

• enable sqllab in database configuration
• open sqllab
• type invalid sql
• click save as dataset

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

[qleroy]

Tested on my dev setup and confirm the Fatal error is turned into a Dataset could not be created. error.

This is also a first step towards resolving #25786.

[john-bodley]

Thanks @jzhao62 for the PR. Would you mind also adding some unit tests to prevent potential future regressions?

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 69.81%. Comparing base (5b1d6b2) to head (f0ddd44). Report is 2 commits behind head on master.

Files	Patch %	Lines
superset/connectors/sqla/utils.py	60.00%	2 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #27796      +/-   ##
==========================================
+ Coverage   67.38%   69.81%   +2.42%     
==========================================
  Files        1920     1920              
  Lines       75242    75246       +4     
  Branches     8423     8423              
==========================================
+ Hits        50705    52536    +1831     
+ Misses      22476    20649    -1827     
  Partials     2061     2061              

Flag	Coverage Δ
hive	48.92% <14.28%> (?)
postgres	77.99% <71.42%> (-0.01%)	:arrow_down:
presto	53.63% <57.14%> (?)
python	83.11% <71.42%> (+5.03%)	:arrow_up:
sqlite	77.43% <71.42%> (-0.01%)	:arrow_down:
unit	56.77% <14.28%> (?)

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[john-bodley]

@jzhao62 for running tests locally try:

> python3.9 -m venv venv
> source venv/bin/activate
> tox -e py39 -- tests/unit_tests/<file>
> tox -e py39-sqlite -- tests/integration_tests/<file>

these instructions should be outlined in CONTRIBUTING.md. Also see here with regards to how to run specific test classes or methods.

[rusackas]

Hey all,

Just wondering if there's any hope of getting this across the finish line, as it would close both an open Discussion and an open Issue.

[jzhao62]

hey @rusackas, i somewhat get stuck in the testing phase, where i cannot pass the celery integration tests from machine, i will address the conflicts and continue work on this