aminghadersohi commented on code in PR #40448:
URL: https://github.com/apache/superset/pull/40448#discussion_r3311294523


##########
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:
   `(metric.get("label") or "").strip()` raises `AttributeError` if a client 
sends `"label": 123` (non-string). In `schema_validator.py` this check runs on 
raw dict input before Pydantic coercion, so non-string types can arrive here.
   
   Suggested fix:
   ```python
   label = metric.get("label")
   if metric.get("sql_expression") and not (isinstance(label, str) and 
label.strip()):
   ```



-- 
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]

Reply via email to