aminghadersohi commented on code in PR #39922:
URL: https://github.com/apache/superset/pull/39922#discussion_r3305703374
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -196,6 +196,29 @@ def _validate_update_against_dataset(
}
)
+ # Column existence + fuzzy-match validation
+ # (mirrors generate_chart pipeline layer 2)
+ from superset.mcp_service.chart.validation.dataset_validator import
DatasetValidator
+
+ is_col_valid, col_error = DatasetValidator.validate_against_dataset(
+ parsed_config, dataset.id
Review Comment:
Fixed in 4c2bd8d — same issue as the adjacent comment. Removed the explicit
`validate_against_dataset` call from `_validate_update_against_dataset`.
`validate_and_compile` (see `compile.py:427-431`) already builds dataset
context from the ORM object already in scope and runs the same Tier 1 check, so
there is no redundant DB query.
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -390,6 +413,24 @@ async def update_chart( # noqa: C901
# config is already a typed ChartConfig | None (validated by Pydantic)
parsed_config = request.config
+ # Normalize column case to match dataset canonical names
+ # (mirrors generate_chart pipeline layer 4)
+ chart_datasource_id = getattr(chart, "datasource_id", None)
+ if parsed_config is not None and chart_datasource_id is not None:
+ from superset.mcp_service.chart.validation.dataset_validator
import (
+ DatasetValidator,
+ NORMALIZATION_EXCEPTIONS,
+ )
+
+ try:
+ parsed_config = DatasetValidator.normalize_column_names(
+ parsed_config, chart.datasource_id
+ )
+ except NORMALIZATION_EXCEPTIONS as e:
+ logger.warning(
+ "Column normalization failed for chart %s: %s", chart.id, e
+ )
Review Comment:
Valid observation. Column normalization in `update_chart` does not yet have
dedicated unit tests. Will add coverage in a follow-up — the existing
`TestUpdateChartValidationGate` suite is the right place to add cases that
verify normalized vs. un-normalized column names and the graceful-exception
path.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
