Skip to content

Commit 345a93e

Browse files
sadpandajoeclaude
andcommitted
fix(explore): show SQL and error for parsing failures in View Query modal
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>
1 parent bf4a66e commit 345a93e

4 files changed

Lines changed: 125 additions & 22 deletions

File tree

superset-frontend/src/explore/components/controls/ViewQueryModal.test.tsx

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ afterEach(() => {
3535

3636
test('renders Alert component when query result contains validation error', async () => {
3737
/**
38-
* Regression test for issue #35492
38+
* Regression test for issue #35492 - Phase 1
3939
* Verifies that validation errors from the backend are displayed in an Alert
4040
* component instead of showing a blank panel
4141
*/
@@ -49,7 +49,9 @@ test('renders Alert component when query result contains validation error', asyn
4949
],
5050
});
5151

52-
render(<ViewQueryModal latestQueryFormData={mockFormData} />);
52+
render(<ViewQueryModal latestQueryFormData={mockFormData} />, {
53+
useRedux: true,
54+
});
5355

5456
// Wait for API call to complete
5557
await waitFor(() =>
@@ -60,3 +62,47 @@ test('renders Alert component when query result contains validation error', asyn
6062
expect(screen.getByRole('alert')).toBeInTheDocument();
6163
expect(screen.getByText('Missing temporal column')).toBeInTheDocument();
6264
});
65+
66+
test('renders both Alert and SQL query when parsing error occurs', async () => {
67+
/**
68+
* Regression test for issue #35492 - Phase 2
69+
* Verifies that parsing errors (which occur after SQL generation) display
70+
* both the error message AND the SQL query that failed to parse.
71+
*
72+
* This differs from validation errors (Phase 1) where no SQL was generated.
73+
* For parsing errors, the SQL was successfully compiled but optimization failed.
74+
*/
75+
// Mock API response with parsing error (has both query and error)
76+
fetchMock.post(chartDataEndpoint, {
77+
result: [
78+
{
79+
query: 'SELECT SUM ( Open',
80+
error: "Error parsing near 'Open' at line 1:17",
81+
language: 'sql',
82+
},
83+
],
84+
});
85+
86+
render(<ViewQueryModal latestQueryFormData={mockFormData} />, {
87+
useRedux: true,
88+
});
89+
90+
// Wait for the error message to appear
91+
await waitFor(() =>
92+
expect(
93+
screen.getByText("Error parsing near 'Open' at line 1:17"),
94+
).toBeInTheDocument(),
95+
);
96+
97+
// Assert Alert component is rendered with error message
98+
expect(screen.getByRole('alert')).toBeInTheDocument();
99+
100+
// Assert SQL query is also displayed
101+
// Note: The SQL is rendered inside a syntax-highlighted code block where
102+
// each keyword is in a separate span element
103+
await waitFor(() => {
104+
expect(screen.getByText('SELECT')).toBeInTheDocument();
105+
expect(screen.getByText('SUM')).toBeInTheDocument();
106+
expect(screen.getByText('Open')).toBeInTheDocument();
107+
});
108+
});

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

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import { FC, useEffect, useState } from 'react';
19+
import { FC, Fragment, useEffect, useState } from 'react';
2020

2121
import {
2222
styled,
@@ -89,23 +89,26 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData }) => {
8989

9090
return (
9191
<ViewQueryModalContainer>
92-
{result.map((item, index) =>
93-
item.error ? (
94-
<Alert
95-
key={`error-${index}`}
96-
type="error"
97-
message={item.error}
98-
closable={false}
99-
/>
100-
) : item.query ? (
101-
<ViewQuery
102-
key={`query-${index}`}
103-
datasource={latestQueryFormData.datasource}
104-
sql={item.query}
105-
language={item.language}
106-
/>
107-
) : null,
108-
)}
92+
{result.map((item, index) => (
93+
<Fragment key={index}>
94+
{item.error && (
95+
<Alert
96+
key={`error-${index}`}
97+
type="error"
98+
message={item.error}
99+
closable={false}
100+
/>
101+
)}
102+
{item.query && (
103+
<ViewQuery
104+
key={`query-${index}`}
105+
datasource={latestQueryFormData.datasource}
106+
sql={item.query}
107+
language={item.language}
108+
/>
109+
)}
110+
</Fragment>
111+
))}
109112
</ViewQueryModalContainer>
110113
);
111114
};

superset/common/query_actions.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
from superset.common.chart_data import ChartDataResultType
2525
from superset.common.db_query_status import QueryStatus
2626
from superset.connectors.sqla.models import BaseDatasource
27-
from superset.exceptions import QueryObjectValidationError
27+
from superset.exceptions import QueryObjectValidationError, SupersetParseError
2828
from superset.utils.core import (
2929
extract_column_dtype,
3030
extract_dataframe_dtypes,
@@ -86,7 +86,15 @@ def _get_query(
8686
try:
8787
result["query"] = datasource.get_query_str(query_obj.to_dict())
8888
except QueryObjectValidationError as err:
89+
# Validation errors (missing required fields, invalid config)
90+
# No SQL was generated
8991
result["error"] = err.message
92+
except SupersetParseError as err:
93+
# Parsing errors (SQL optimization/parsing failed)
94+
# SQL was generated but couldn't be optimized - show both
95+
if err.error.extra:
96+
result["query"] = err.error.extra.get("sql")
97+
result["error"] = err.error.message
9098
return result
9199

92100

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

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18-
from unittest.mock import Mock
18+
from unittest.mock import Mock, patch
1919

2020
import pytest
2121

@@ -207,3 +207,49 @@ def test_full_result_type_fails_fast_on_first_error_in_multiple_queries() -> Non
207207
command.run()
208208

209209
assert "First query failed" in str(exc_info.value)
210+
211+
212+
def test_get_query_catches_parsing_error() -> None:
213+
"""
214+
Test that _get_query() catches SupersetParseError and returns both SQL and error.
215+
216+
When SQL generation succeeds but optimization/parsing fails:
217+
- SQL has already been compiled (stored in error.extra['sql'])
218+
- Error message describes the parsing failure
219+
- Both should be returned to the frontend for display
220+
221+
This is Phase 2 of the fix for GitHub Issue #35492.
222+
Phase 1 fixed validation errors (before SQL generation).
223+
Phase 2 fixes parsing errors (after SQL generation, during optimization).
224+
"""
225+
from superset.common.query_actions import _get_query
226+
from superset.common.query_object import QueryObject
227+
from superset.exceptions import SupersetParseError
228+
229+
# Create mock query_context and query_obj
230+
mock_query_context = Mock()
231+
mock_query_obj = Mock(spec=QueryObject)
232+
233+
# Create SupersetParseError with SQL in error.extra
234+
parse_error = SupersetParseError(
235+
sql="SELECT SUM ( Open", # SQL was generated before parsing failed
236+
engine="postgresql",
237+
message="Error parsing near 'Open' at line 1:17",
238+
line=1,
239+
column=17,
240+
)
241+
242+
# Mock _get_datasource and datasource.get_query_str to raise the error
243+
with patch("superset.common.query_actions._get_datasource") as mock_get_ds:
244+
mock_datasource = Mock()
245+
mock_datasource.query_language = "sql"
246+
mock_datasource.get_query_str.side_effect = parse_error
247+
mock_get_ds.return_value = mock_datasource
248+
249+
# GREEN: Exception is caught, values returned (new behavior after fix)
250+
result = _get_query(mock_query_context, mock_query_obj, False)
251+
252+
# Should return both query (from error.extra['sql']) and error message
253+
assert result["query"] == "SELECT SUM ( Open"
254+
assert result["error"] == "Error parsing near 'Open' at line 1:17"
255+
assert result["language"] == "sql"

0 commit comments

Comments
 (0)