Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { screen, render, waitFor } from 'spec/helpers/testing-library';
import fetchMock from 'fetch-mock';
import ViewQueryModal from './ViewQueryModal';

const mockFormData = {
datasource: '1__table',
viz_type: 'table',
};

const chartDataEndpoint = 'glob:*/api/v1/chart/data*';

afterEach(() => {
jest.resetAllMocks();
fetchMock.restore();
});

test('renders Alert component when query result contains validation error', async () => {
/**
* Regression test for issue #35492 - Phase 1
* Verifies that validation errors from the backend are displayed in an Alert
* component instead of showing a blank panel
*/
// Mock API response with validation error
fetchMock.post(chartDataEndpoint, {
result: [
{
error: 'Missing temporal column',
language: 'sql',
},
],
});

render(<ViewQueryModal latestQueryFormData={mockFormData} />, {
useRedux: true,
});

// Wait for API call to complete
await waitFor(() =>
expect(fetchMock.calls(chartDataEndpoint)).toHaveLength(1),
);

// Assert Alert component is rendered with error message
expect(screen.getByRole('alert')).toBeInTheDocument();
expect(screen.getByText('Missing temporal column')).toBeInTheDocument();
});

test('renders both Alert and SQL query when parsing error occurs', async () => {
/**
* Regression test for issue #35492 - Phase 2
* Verifies that parsing errors (which occur after SQL generation) display
* both the error message AND the SQL query that failed to parse.
*
* This differs from validation errors (Phase 1) where no SQL was generated.
* For parsing errors, the SQL was successfully compiled but optimization failed.
*/
// Mock API response with parsing error (has both query and error)
fetchMock.post(chartDataEndpoint, {
result: [
{
query: 'SELECT SUM ( Open',
error: "Error parsing near 'Open' at line 1:17",
language: 'sql',
},
],
});

render(<ViewQueryModal latestQueryFormData={mockFormData} />, {
useRedux: true,
});

// Wait for the error message to appear
await waitFor(() =>
expect(
screen.getByText("Error parsing near 'Open' at line 1:17"),
).toBeInTheDocument(),
);

// Assert Alert component is rendered with error message
expect(screen.getByRole('alert')).toBeInTheDocument();

// Assert SQL query is also displayed
// Note: The SQL is rendered inside a syntax-highlighted code block where
// each keyword is in a separate span element
await waitFor(() => {
expect(screen.getByText('SELECT')).toBeInTheDocument();
expect(screen.getByText('SUM')).toBeInTheDocument();
expect(screen.getByText('Open')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,17 @@
* specific language governing permissions and limitations
* under the License.
*/
import { FC, useEffect, useState } from 'react';
import { FC, Fragment, useEffect, useState } from 'react';

import {
ensureIsArray,
t,
getClientErrorObject,
QueryFormData,
} from '@superset-ui/core';
import { styled } from '@apache-superset/core/ui';
import { styled, Alert } from '@apache-superset/core/ui';
import { Loading } from '@superset-ui/core/components';
import { SupportedLanguage } from '@superset-ui/core/components/CodeSyntaxHighlighter';
import { getChartDataRequest } from 'src/components/Chart/chartAction';
import ViewQuery from 'src/explore/components/controls/ViewQuery';

Expand All @@ -34,8 +35,9 @@ interface Props {
}

type Result = {
query: string;
language: string;
query?: string;
language: SupportedLanguage;
error?: string;
};

const ViewQueryModalContainer = styled.div`
Expand Down Expand Up @@ -87,16 +89,21 @@ const ViewQueryModal: FC<Props> = ({ latestQueryFormData }) => {

return (
<ViewQueryModalContainer>
{result.map((item, index) =>
item.query ? (
<ViewQuery
key={`query-${index}`}
datasource={latestQueryFormData.datasource}
sql={item.query}
language="sql"
/>
) : null,
)}
{result.map((item, index) => (
// Static API response data - index is appropriate for keys
<Fragment key={index}>
{item.error && (
<Alert type="error" message={item.error} closable={false} />
)}
{item.query && (
<ViewQuery
datasource={latestQueryFormData.datasource}
sql={item.query}
language={item.language}
/>
)}
</Fragment>
))}
</ViewQueryModalContainer>
);
};
Expand Down
9 changes: 6 additions & 3 deletions superset/charts/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -1482,9 +1482,12 @@ class ChartDataResponseResult(Schema):
allow_none=None,
)
query = fields.String(
metadata={"description": "The executed query statement"},
required=True,
allow_none=False,
metadata={
"description": "The executed query statement. May be absent when "
"validation errors occur."
},
required=False,
allow_none=True,
)
status = fields.String(
metadata={"description": "Status of the query"},
Expand Down
8 changes: 7 additions & 1 deletion superset/commands/chart/data/get_data_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
ChartDataCacheLoadError,
ChartDataQueryFailedError,
)
from superset.common.chart_data import ChartDataResultType
from superset.common.query_context import QueryContext
from superset.exceptions import CacheLoadError

Expand All @@ -48,8 +49,13 @@ def run(self, **kwargs: Any) -> dict[str, Any]:
except CacheLoadError as ex:
raise ChartDataCacheLoadError(ex.message) from ex

# Skip error check for query-only requests - errors are returned in payload
# This allows View Query modal to display validation errors
for query in payload["queries"]:
if query.get("error"):
if (
query.get("error")
and self._query_context.result_type != ChartDataResultType.QUERY
):
raise ChartDataQueryFailedError(
_("Error: %(error)s", error=query["error"])
)
Expand Down
10 changes: 9 additions & 1 deletion superset/common/query_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from superset.common.chart_data import ChartDataResultType
from superset.common.db_query_status import QueryStatus
from superset.connectors.sqla.models import BaseDatasource
from superset.exceptions import QueryObjectValidationError
from superset.exceptions import QueryObjectValidationError, SupersetParseError
from superset.utils.core import (
extract_column_dtype,
extract_dataframe_dtypes,
Expand Down Expand Up @@ -86,7 +86,15 @@ def _get_query(
try:
result["query"] = datasource.get_query_str(query_obj.to_dict())
except QueryObjectValidationError as err:
# Validation errors (missing required fields, invalid config)
# No SQL was generated
result["error"] = err.message
except SupersetParseError as err:
# Parsing errors (SQL optimization/parsing failed)
# SQL was generated but couldn't be optimized - show both
if err.error.extra and (sql := err.error.extra.get("sql")) is not None:
result["query"] = sql
result["error"] = err.error.message
return result


Expand Down
16 changes: 16 additions & 0 deletions tests/unit_tests/charts/commands/data/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
Loading
Loading