fix(mssql): avoid trying to return a resultset for DML queries with not resultset

[Yuval-Moshe]

SUMMARY

Unlike other db engines, the mssql one does not include a check to see if there is even a need to return a result set before trying to fetch the data, that's why we are failing on a non select queries with the Statement not executed or executed statement has no resultset error.
In order to see if there is any result to try and fetch, we should check if the cursor description is not None.

Just for a reference we can see that other db engines like oracle or postgres do have that check before trying to fetch:
Oracle
https://github.com/apache/superset/blob/85a7d5cb3ebe833cfc2980f0846f15bb7ce1dd01/superset/db_engine_specs/postgres.py#L176C1-L179

Postgres
https://github.com/apache/superset/blob/85a7d5cb3ebe833cfc2980f0846f15bb7ce1dd01/superset/db_engine_specs/postgres.py#L176C1-L179

The result is failure in running a DML queries due to 'mssql error: Statement not executed or executed statement has no resultset'

So the fix is simply to add that check before calling the inherited fetch_data method.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Run a DML query via the SQL Lab on an mssql DB

ADDITIONAL INFORMATION

• Has associated issue: Can't run UPDATE via SqlLab (mssql) with both pymssql and pyodbc: "Statement not executed or executed statement has no resultset" #24810

[github-actions]

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

[john-bodley]

Thanks @Yuval-Moshe for the fix. Would you mind looking into why the unit tests are failing? Additionally adding a unit test for this would be greatly appreciated as it would help prevent future regressions if said logic changed.

[Yuval-Moshe]

Hi @john-bodley, sure no problem.
I've fixed the failing test be providing a mocked cursor instead of a None cursor, and added an additional unit test to test the no description scenario.

[michael-s-molina]

Thank you for the improvement @Yuval-Moshe!

[Yuval-Moshe]

@michael-s-molina Sure no problem!
What else is needed for this PR to be merged? I see you approved the PR and all the checks passed successfully, are we pending for @betodealmeida to review also?

[Yuval-Moshe]

@michael-s-molina Thanks!
Can you tell when a new helm chart release will be pushed? or where I can track it?

[michael-s-molina]

@michael-s-molina Thanks! Can you tell when a new helm chart release will be pushed? or where I can track it?

Helm files are maintained by @craig-rueda. I think a Helm release will be available once 3.0 is out.