fix(mcp): convert adhoc filters to QueryObject format before query compilation (#38774)

This commit is contained in:
Amin Ghadersohi
2026-03-20 15:43:09 -04:00
committed by GitHub
parent 0d5721910e
commit 44c2c765ae
4 changed files with 87 additions and 2 deletions

View File

@@ -345,6 +345,27 @@ def _add_adhoc_filters(
]
def adhoc_filters_to_query_filters(
adhoc_filters: list[Dict[str, Any]],
) -> list[Dict[str, Any]]:
"""Convert adhoc filter format to QueryObject filter format.
Adhoc filters use ``{subject, operator, comparator}`` keys while
``QueryContextFactory`` expects ``{col, op, val}`` (QueryObjectFilterClause).
"""
result: list[Dict[str, Any]] = []
for f in adhoc_filters:
if f.get("expressionType") == "SIMPLE":
result.append(
{
"col": f.get("subject"),
"op": f.get("operator"),
"val": f.get("comparator"),
}
)
return result
def map_table_config(config: TableChartConfig) -> Dict[str, Any]:
"""Map table chart config to form_data with defensive validation."""
# Early validation to prevent empty charts

View File

@@ -81,11 +81,17 @@ def generate_preview_from_form_data(
# Create query context from form data using factory
from superset.common.query_context_factory import QueryContextFactory
from superset.mcp_service.chart.chart_utils import (
adhoc_filters_to_query_filters,
)
# Build columns list: include x_axis and groupby for XY charts,
# fall back to form_data "columns" for table charts
columns = _build_query_columns(form_data)
query_filters = adhoc_filters_to_query_filters(
form_data.get("adhoc_filters", [])
)
factory = QueryContextFactory()
query_context_obj = factory.create(
datasource={"id": dataset_id, "type": "table"},
@@ -95,7 +101,7 @@ def generate_preview_from_form_data(
"metrics": form_data.get("metrics", []),
"orderby": form_data.get("orderby", []),
"row_limit": form_data.get("row_limit", 100),
"filters": form_data.get("adhoc_filters", []),
"filters": query_filters,
"time_range": form_data.get("time_range", "No filter"),
}
],

View File

@@ -81,10 +81,14 @@ def _compile_chart(
ChartDataQueryFailedError,
)
from superset.common.query_context_factory import QueryContextFactory
from superset.mcp_service.chart.chart_utils import adhoc_filters_to_query_filters
from superset.mcp_service.chart.preview_utils import _build_query_columns
try:
columns = _build_query_columns(form_data)
query_filters = adhoc_filters_to_query_filters(
form_data.get("adhoc_filters", [])
)
factory = QueryContextFactory()
query_context = factory.create(
datasource={"id": dataset_id, "type": "table"},
@@ -94,7 +98,7 @@ def _compile_chart(
"metrics": form_data.get("metrics", []),
"orderby": form_data.get("orderby", []),
"row_limit": 2,
"filters": form_data.get("adhoc_filters", []),
"filters": query_filters,
"time_range": form_data.get("time_range", "No filter"),
}
],

View File

@@ -24,6 +24,7 @@ import pytest
from superset.mcp_service.chart.chart_utils import (
_add_adhoc_filters,
adhoc_filters_to_query_filters,
configure_temporal_handling,
create_metric_object,
generate_chart_name,
@@ -1447,3 +1448,56 @@ class TestValidateChartDataset:
mock_find.side_effect = SQLAlchemyError("db gone")
url = generate_explore_link(5, {"viz_type": "table"})
assert "datasource_id=5" in url
class TestAdhocFiltersToQueryFilters:
"""Tests for adhoc_filters_to_query_filters conversion."""
def test_converts_simple_filters(self) -> None:
adhoc = [
{
"clause": "WHERE",
"expressionType": "SIMPLE",
"subject": "genre",
"operator": "==",
"comparator": "Action",
}
]
result = adhoc_filters_to_query_filters(adhoc)
assert result == [{"col": "genre", "op": "==", "val": "Action"}]
def test_converts_multiple_filters(self) -> None:
adhoc = [
{
"clause": "WHERE",
"expressionType": "SIMPLE",
"subject": "genre",
"operator": "==",
"comparator": "Action",
},
{
"clause": "WHERE",
"expressionType": "SIMPLE",
"subject": "year",
"operator": ">=",
"comparator": "2010",
},
]
result = adhoc_filters_to_query_filters(adhoc)
assert len(result) == 2
assert result[0] == {"col": "genre", "op": "==", "val": "Action"}
assert result[1] == {"col": "year", "op": ">=", "val": "2010"}
def test_empty_list(self) -> None:
assert adhoc_filters_to_query_filters([]) == []
def test_skips_non_simple_expression_types(self) -> None:
adhoc = [
{
"clause": "WHERE",
"expressionType": "SQL",
"sqlExpression": "col > 5",
}
]
result = adhoc_filters_to_query_filters(adhoc)
assert result == []