fix(explore): show validation errors in View Query modal by sadpandajoe · Pull Request #35969 · apache/superset

sadpandajoe · 2025-11-04T05:54:48Z

SUMMARY

Fixes #35492 - Restores the ability to view helpful information in the "View Query" modal when chart queries fail.

This PR addresses a regression introduced during the 2023 command reorganization (commit 07bcfa9) that broke error handling for the View Query feature. Previously in Superset 4.1.3, when charts failed, users could view either SQL or error messages. The reorganization caused all errors to return 400 status codes, making the frontend unable to display any helpful information.

Two types of errors are now properly handled:

Validation errors (Phase 1): Occur before SQL generation (e.g., missing required fields, invalid metrics)
- Before: Blank modal or generic "Sorry, An error occurred" message
- After: Clear error message displayed in Alert component
Parsing errors (Phase 2): Occur after SQL generation during optimization (e.g., SQL syntax issues)
- Before: 500 error, SQL lost completely
- After: Both error message AND SQL displayed together

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After

TESTING INSTRUCTIONS

Automated Tests

# Backend unit tests
pytest tests/unit_tests/charts/commands/data/test_get_data_command.py -v

# Frontend component test
npm run test -- ViewQueryModal.test.tsx

Manual Testing

Test Case 1: Validation Error (The Fix)

Create a chart in Explore view
Add duplicate metric labels (e.g., add "sum__num" twice)
Click "View Query" button
Expected: Red Alert shows "Duplicate column/metric labels: sum__num"
Before fix: Blank panel or generic error

Test Case 2: Valid Chart (No Regression)

Create a valid chart
Click "View Query"
Expected: SQL query displayed normally
Verify: No regression on happy path

Test Case 3: Database Error (No Regression)

Create a chart with invalid column in database
Click "View Query"
Expected: Shows SQL query (generated before execution error)
Verify: Database errors still work

ADDITIONAL INFORMATION

Has associated issue: Fixes "View query" on a chart whose query errored shows error message, not query (6.0.0rc2) #35492
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

bito-code-review · 2025-11-04T05:54:58Z

Code Review Agent Run #a4a403

Actionable Suggestions - 0

Additional Suggestions - 5

tests/unit_tests/charts/commands/data/test_get_data_command.py - 3

Missing module docstring in test file · Line 1-1

Add a module-level docstring to describe the purpose of this test file. This helps with code documentation and maintainability.

Code suggestion

 @@ -17,6 +17,10 @@
-# under the License.
-
-from unittest.mock import Mock
+# under the License.
+"""
+Unit tests for ChartDataCommand functionality.
+"""
+
+from unittest.mock import Mock

Docstring formatting needs blank line separation · Line 29-29

Add a blank line between the summary line and description in the docstring. This issue also appears in functions at lines 61, 84, 106, 129, 152, and 188.

Code suggestion

 @@ -28,7 +28,8 @@
-def test_query_result_type_allows_validation_error_payload() -> None:
-    """
-    Regression test: Ensure result_type='query' with error payload returns
-    the error instead of raising ChartDataQueryFailedError.
+def test_query_result_type_allows_validation_error_payload() -> None:
+    """Regression test: Ensure result_type='query' with error payload returns the error.
+
+    Instead of raising ChartDataQueryFailedError, validation errors should pass through.

Missing trailing comma in dictionary literal · Line 46-46

Add trailing comma after the dictionary entry. This issue also appears at lines 71, 93, 115, 138, 166, and 200.

Code suggestion

 @@ -45,7 +45,7 @@
-    mock_query_context.get_payload.return_value = {
-        "queries": [{"error": "Missing temporal column", "language": "sql"}]
-    }
+    mock_query_context.get_payload.return_value = {
+        "queries": [{"error": "Missing temporal column", "language": "sql"}],
+    }

tests/unit_tests/charts/commands/data/__init__.py - 1
- Missing docstring in public package · Line 1-1
  
  The `__init__.py` file is missing a docstring. Consider adding a module-level docstring to document the package's purpose and contents.
  Code suggestion
  @@ -16,0 +17,3 @@ # under the License. +""" +Unit tests for chart data commands. +"""

superset/charts/schemas.py - 1

Missing trailing comma in tuple · Line 1487-1487

Missing trailing comma after `allow_none=None` on line 1487. This violates the COM812 rule and should be fixed for consistency with the codebase style.

Code suggestion

 @@ -1479,9 +1479,9 @@
-    is_cached = fields.Boolean(
-        metadata={"description": "Is the result cached"},
-        required=True,
-        allow_none=None,
-    )
+    is_cached = fields.Boolean(
+        metadata={"description": "Is the result cached"},
+        required=True,
+        allow_none=None,
+    )

Review Details

Files reviewed - 6 · Commit Range: f04b81e..bf4a66e
- superset-frontend/src/explore/components/controls/ViewQueryModal.test.tsx
- superset-frontend/src/explore/components/controls/ViewQueryModal.tsx
- superset/charts/schemas.py
- superset/commands/chart/data/get_data_command.py
- tests/unit_tests/charts/commands/data/__init__.py
- tests/unit_tests/charts/commands/data/test_get_data_command.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Negative logic in result type check may bypass error handling ▹ view
	Potential undefined language prop ▹ view
	Missing validation for query field on successful status ▹ view

Files scanned

File Path	Reviewed
superset/commands/chart/data/get_data_command.py	✅
superset-frontend/src/explore/components/controls/ViewQueryModal.tsx	✅
superset/charts/schemas.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

github-actions · 2025-11-04T06:05:02Z

🎪 Showtime is building environment on GHA for bf4a66e

github-actions · 2025-11-04T06:19:01Z

🎪 Showtime deployed environment on GHA for bf4a66e

• Environment: http://35.92.249.128:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

github-actions · 2025-11-06T00:19:46Z

🎪 Showtime is building environment on GHA for 345a93e

github-actions · 2025-11-07T01:30:00Z

🎪 Showtime deployed environment on GHA for 565fcfb

• Environment: http://18.236.166.70:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

github-actions · 2025-11-07T02:36:11Z

🎪 Showtime is building environment on GHA for 2f41fce

github-actions · 2025-11-07T02:44:24Z

🎪 Showtime deployed environment on GHA for 2f41fce

• Environment: http://52.40.60.56:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

Copilot

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

github-actions · 2025-11-07T16:59:56Z

🎪 Showtime is building environment on GHA for 3f34a81

github-actions · 2025-11-07T17:14:43Z

🎪 Showtime deployed environment on GHA for 3f34a81

• Environment: http://44.244.205.125:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

When chart queries fail due to validation errors (e.g., missing required fields, invalid metrics), the View Query modal now displays helpful error messages instead of showing a blank panel. This restores functionality that was broken in 2023 when ChartDataCommand was added. The command raised exceptions for ALL errors, but should only fail-fast for data execution requests, not query-only requests. Changes: - Backend: Skip error check in ChartDataCommand for result_type='query' - Schema: Make query field optional (validation errors return error without SQL) - Frontend: Display Alert component for errors, use item.language from backend - Tests: Add 7 unit tests including regression test to prevent future breakage Fixes #35492 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

Add React Testing Library test to verify that ViewQueryModal correctly displays validation errors in an Alert component when the backend returns errors in the query result. This test ensures the fix for issue #35492 continues to work correctly by verifying the frontend properly renders error messages instead of showing a blank panel. Test coverage: - Mocks chart/data API to return validation error - Verifies Alert component is rendered - Verifies error message is displayed Related to #35492 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

…odal When SQL generation succeeds but optimization/parsing fails, View Query modal now displays both the error message and the SQL that failed to parse. This is Phase 2 of the fix for #35492. Phase 1 fixed validation errors (before SQL generation). Phase 2 fixes parsing errors (after SQL generation, during optimization). Backend changes: - Added SupersetParseError handling in _get_query() - Extracts SQL from error.extra['sql'] when parsing fails - Returns both query and error to frontend Frontend changes: - Updated ViewQueryModal to render both Alert and ViewQuery simultaneously - Changed from ternary (either/or) to conditional rendering (both) - Added Fragment wrapper to support multiple children Tests: - Added backend unit test for SupersetParseError handling - Added frontend test for rendering both error and SQL - All 8 backend tests passing - All 2 frontend tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

…ery modal Addresses PR feedback for safer SQL extraction from parsing errors: - Use explicit None check instead of truthiness to preserve falsy SQL like "" - Split edge case tests into separate functions for better granularity - Remove time-specific references from test docstrings Changes: - query_actions.py: Check `is not None` to avoid discarding valid empty SQL - test_get_data_command.py: Split combined test into two independent tests - test_get_query_handles_parsing_error_with_missing_sql_key - test_get_query_handles_parsing_error_with_null_sql_value 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

Addresses PR feedback: - Remove redundant keys from Alert and ViewQuery components (Fragment already has key) - Improve Fragment key to use content-based identifiers (query/error content) instead of array index This follows React best practices by: 1. Avoiding duplicate keys on conditionally rendered children 2. Using stable, content-based keys instead of index-based keys 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2025-11-07T17:38:21Z

🎪 Showtime is building environment on GHA for 7620bc8

Copilot

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Copilot · 2025-11-07T17:42:29Z

+        // Use content-based key when available, fall back to index
+        const key = item.query || item.error || `result-${index}`;
+        return (
+          <Fragment key={key}>


Using query or error text as a React key could cause issues if the same error/query appears multiple times, or if very long SQL queries are used as keys. Consider using a more stable identifier or always use the index-based key pattern.

Suggested change

// Use content-based key when available, fall back to index

const key = item.query || item.error || `result-${index}`;

return (

<Fragment key={key}>

// Use index as key to avoid issues with duplicate or long query/error strings

return (

<Fragment key={index}>

github-actions · 2025-11-07T17:50:31Z

🎪 Showtime deployed environment on GHA for 7620bc8

• Environment: http://52.24.34.130:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

github-actions · 2025-11-07T19:20:59Z

🎪 Showtime is building environment on GHA for 5030db3

github-actions · 2025-11-07T19:31:39Z

🎪 Showtime deployed environment on GHA for 5030db3

• Environment: http://35.95.73.104:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

…ebase patterns Changed from content-based keys (item.query || item.error) to index-based keys for the Fragment elements in ViewQueryModal. This aligns with established Superset patterns (e.g., MarshmallowErrorMessage.tsx) for displaying static API response data. Both approaches are technically correct for static data, but index-based is simpler and more consistent. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2025-11-07T21:39:48Z

🎪 Showtime is building environment on GHA for add009b

github-actions · 2025-11-07T21:49:01Z

🎪 Showtime deployed environment on GHA for add009b

• Environment: http://35.166.36.14:8080 (admin/admin)
• Lifetime: 48h auto-cleanup
• Updates: New commits create fresh environments automatically

Copilot

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

pull-request-size Bot added the size/L label Nov 4, 2025

dosubot Bot added the explore Namespace | Anything related to Explore label Nov 4, 2025

korbit-ai Bot suggested changes Nov 4, 2025

View reviewed changes

Comment thread superset/commands/chart/data/get_data_command.py

Comment thread superset-frontend/src/explore/components/controls/ViewQueryModal.tsx Outdated

Comment thread superset/charts/schemas.py

sadpandajoe added the 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR label Nov 4, 2025

github-actions Bot added 🎪 bf4a66e 🚦 building and removed 🎪 ⚡ showtime-trigger-start Create new ephemeral environment for this PR labels Nov 4, 2025

github-actions Bot added 🎪 bf4a66e 🚦 deploying and removed 🎪 bf4a66e 🚦 building labels Nov 4, 2025

sadpandajoe added the review:draft label Nov 4, 2025

sadpandajoe marked this pull request as draft November 4, 2025 22:27

github-actions Bot added 🎪 345a93e 🚦 building labels Nov 6, 2025

github-actions Bot removed 🎪 345a93e 🌐 35.90.244.30:8080 labels Nov 7, 2025

sadpandajoe force-pushed the issue-35492-fix-show-query-with-error branch from 565fcfb to 2f41fce Compare November 7, 2025 02:35

github-actions Bot added 🎪 2f41fce 🚦 building labels Nov 7, 2025

Copilot AI reviewed Nov 7, 2025

View reviewed changes

Comment thread superset-frontend/src/explore/components/controls/ViewQueryModal.tsx

sadpandajoe and others added 5 commits November 7, 2025 09:16

Copilot AI reviewed Nov 7, 2025

View reviewed changes

Copilot AI reviewed Nov 10, 2025

View reviewed changes

betodealmeida approved these changes Nov 10, 2025

View reviewed changes

dosubot Bot mentioned this pull request Apr 3, 2026

SQL Lab view query looks bad #39076

Closed

3 tasks

Conversation

sadpandajoe commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Automated Tests

Manual Testing

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #a4a403

Uh oh!

korbit-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Nov 4, 2025

Uh oh!

github-actions Bot commented Nov 4, 2025

Uh oh!

github-actions Bot commented Nov 6, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

github-actions Bot commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sadpandajoe commented Nov 4, 2025 •

edited

Loading

bito-code-review Bot commented Nov 4, 2025 •

edited

Loading

korbit-ai Bot left a comment •

edited

Loading