gabotorresruiz commented on code in PR #40448:
URL: https://github.com/apache/superset/pull/40448#discussion_r3311328103
##########
superset/mcp_service/chart/validation/schema_validator.py:
##########
@@ -392,22 +392,42 @@ def _pre_validate_big_number_config(
],
error_code="INVALID_BIG_NUMBER_METRIC_TYPE",
)
- if not metric.get("aggregate") and not metric.get("saved_metric"):
+ if (
+ not metric.get("aggregate")
+ and not metric.get("saved_metric")
+ and not metric.get("sql_expression")
+ ):
return False, ChartGenerationError(
error_type="missing_metric_aggregate",
- message="Big Number metric must include an aggregate function "
- "or reference a saved metric",
- details="The metric must have an 'aggregate' field "
- "or 'saved_metric': true",
+ message="Big Number metric must include an aggregate function,
"
+ "a saved metric reference, or a SQL expression",
+ details="The metric must have an 'aggregate' field, "
+ "'saved_metric': true, or 'sql_expression'",
suggestions=[
"Add 'aggregate' to your metric: "
"{'name': 'col', 'aggregate': 'SUM'}",
"Or use a saved metric: "
"{'name': 'total_sales', 'saved_metric': true}",
+ "Or a custom SQL metric: "
+ "{'sql_expression': 'SUM(a)/SUM(b)', 'label': 'Ratio'}",
"Valid aggregates: SUM, COUNT, AVG, MIN, MAX",
],
error_code="MISSING_BIG_NUMBER_AGGREGATE",
)
+ if metric.get("sql_expression") and not (metric.get("label") or
"").strip():
Review Comment:
Good catch... Again! This has been fixed. The `(metric.get("label") or
"").strip()` pattern looks fine when label is `None/empty/string` but raises
`AttributeError` on any other truthy type (`123`, `True`, `[]`, etc.), which a
buggy or hostile client could absolutely send through pre-validation.
Applied your suggested `isinstance(label, str) and label.strip()` form
verbatim, plus a regression test
`test_sql_expression_with_non_string_label_fails_cleanly` that sends `"label":
123` and asserts the validator returns `MISSING_SQL_METRIC_LABEL` cleanly.
--
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]
