You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "I3eka (via GitHub)" <gi...@apache.org> on 2026/08/13 15:58:23 UTC
[PR] fix(mcp): surface virtual dataset SQL errors [superset]
I3eka opened a new pull request, #43119:
URL: https://github.com/apache/superset/pull/43119
# fix(mcp): surface virtual dataset SQL errors
### SUMMARY
Return database validation failures from `create_virtual_dataset` as actionable MCP results instead of unexpected adapter failures.
Base: `apache/superset:master` at `e2bb33b1da17c16a51e54d84bcf496516d67a713`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; backend behavior only.
### TESTING INSTRUCTIONS
`pytest -q tests/unit_tests/mcp_service/dataset/tool/test_dataset_tools.py`
Result: 80 passed.
The changed files also pass the repository pre-commit hooks.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): surface virtual dataset SQL errors [superset]
Posted by "I3eka (via GitHub)" <gi...@apache.org>.
I3eka closed pull request #43119: fix(mcp): surface virtual dataset SQL errors
URL: https://github.com/apache/superset/pull/43119
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): surface virtual dataset SQL errors [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43119:
URL: https://github.com/apache/superset/pull/43119#issuecomment-5283043759
<!-- Bito Reply -->
The review suggestion is correct. Catching `SupersetGenericDBErrorException` broadly masks infrastructure issues (like connection or permission failures) as simple SQL validation errors. To resolve this, you should catch only the specific exception type related to SQL parsing or validation, allowing other database-level exceptions to propagate to the general error handler.
If you have a specific validation exception class (e.g., `SupersetSQLValidationError`), replace the broad catch block with it. If not, you may need to inspect the exception message or error code within the `except` block to determine if it is truly a recoverable validation error before returning a successful tool response.
**superset/mcp_service/dataset/tool/create_virtual_dataset.py**
```
except SupersetSQLValidationError as exc:
await ctx.warning(f"Virtual dataset SQL failed validation: {exc}")
return CreateVirtualDatasetResponse(
id=None,
dataset_name=request.dataset_name,
sql=request.sql,
database_id=request.database_id,
columns=[],
url=None,
error=f"Dataset SQL could not be executed: {exc}",
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): surface virtual dataset SQL errors [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43119:
URL: https://github.com/apache/superset/pull/43119#discussion_r3776984977
##########
superset/mcp_service/dataset/tool/create_virtual_dataset.py:
##########
@@ -213,6 +214,17 @@ async def create_virtual_dataset(
url=None,
error=f"Failed to update dataset metadata (creation rolled back): {exc}",
)
+ except SupersetGenericDBErrorException as exc:
+ await ctx.warning(f"Virtual dataset SQL failed validation: {exc}")
+ return CreateVirtualDatasetResponse(
+ id=None,
+ dataset_name=request.dataset_name,
+ sql=request.sql,
+ database_id=request.database_id,
+ columns=[],
+ url=None,
+ error=f"Dataset SQL could not be executed: {exc}",
Review Comment:
**Suggestion:** Do not catch every `SupersetGenericDBErrorException` as a recoverable SQL-validation result. The metadata execution path wraps connection failures, driver errors, warehouse outages, and permission failures in this same exception, so this handler currently hides genuine infrastructure or access failures, reports them as SQL validation problems, and returns a successful MCP tool result with `id=None`. Catch only the specific parse/validation exception intended to be actionable, while allowing operational database failures to propagate through the unexpected-error path. [error handling]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Connection outages appear as SQL validation failures.
- ❌ Permission failures are mislabeled to MCP clients.
- ⚠️ Operational failures return normal tool responses with id=None.
- ⚠️ Monitoring and callers may miss unexpected database incidents.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=369854767bb04d198dec686931f24dd3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=369854767bb04d198dec686931f24dd3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/tool/create_virtual_dataset.py
**Line:** 217:226
**Comment:**
*Error Handling: Do not catch every `SupersetGenericDBErrorException` as a recoverable SQL-validation result. The metadata execution path wraps connection failures, driver errors, warehouse outages, and permission failures in this same exception, so this handler currently hides genuine infrastructure or access failures, reports them as SQL validation problems, and returns a successful MCP tool result with `id=None`. Catch only the specific parse/validation exception intended to be actionable, while allowing operational database failures to propagate through the unexpected-error path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43119&comment_hash=e9a464dfe09a1344126693f527fe230a291946cb63d4d0e22235e738c960fbb8&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43119&comment_hash=e9a464dfe09a1344126693f527fe230a291946cb63d4d0e22235e738c960fbb8&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): surface virtual dataset SQL errors [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43119:
URL: https://github.com/apache/superset/pull/43119#issuecomment-5282954870
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org