kgabryje commented on code in PR #38955:
URL: https://github.com/apache/superset/pull/38955#discussion_r3009889512
##########
superset/mcp_service/chart/validation/dataset_validator.py:
##########
@@ -426,6 +426,8 @@ def _validate_aggregations(
errors = []
for col_ref in column_refs:
+ if col_ref.saved_metric:
Review Comment:
Fixed. Added `_validate_saved_metrics` that checks saved metric refs
exclusively against `available_metrics`. A column name like `"product_line"`
with `saved_metric=True` now gets a clear error message instead of passing
validation and failing at query time. See 61b126c.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -485,6 +485,19 @@ class ColumnRef(BaseModel):
]
| None
) = Field(None, description="SQL aggregate function")
+ saved_metric: bool = Field(
Review Comment:
Checked — `_check_sql_patterns` only blocks DDL/DML keywords (`DROP`,
`DELETE`, `INSERT`, `UPDATE`, `CREATE`, `ALTER`, `EXEC`). Common metric names
like `count`, `sum`, `avg` pass fine. The `name` field regex pattern
`^[a-zA-Z0-9_][a-zA-Z0-9_\s\-\.]*$` is the real constraint, and it accepts
these names. No change needed here.
--
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]
