bito-code-review[bot] commented on PR #43569:
URL: https://github.com/apache/superset/pull/43569#issuecomment-5438298298
<!-- Bito Reply -->
The issue is valid. The `reject_metric_style_groupby` validator in
`TreemapChartConfig` currently checks for `saved_metric` and `sql_expression`
but fails to reject `aggregate` on `groupby` entries, which are intended to be
dimensions. This allows invalid configurations that are later silently
discarded.
To resolve this, update the `reject_metric_style_groupby` validator in
`superset/mcp_service/chart/schemas.py` to also check for the presence of an
`aggregate` value on `groupby` columns.
```python
@model_validator(mode="after")
def reject_metric_style_groupby(self) -> "TreemapChartConfig":
"""groupby entries are hierarchy dimensions, not metrics."""
for i, col in enumerate(self.groupby or []):
_reject_sql_expression_on_dimension(col, f"groupby[{i}]")
if col.saved_metric:
raise ValueError(
f"groupby[{i}] cannot use saved_metric=True; "
"saved metrics belong in the 'metric' field"
)
if col.aggregate:
raise ValueError(
f"groupby[{i}] cannot use aggregate='{col.aggregate}'; "
"aggregates belong in the 'metric' field"
)
return self
```
Would you like me to check the other comments on this PR and implement fixes
for them as well?
**superset/mcp_service/chart/schemas.py**
```
if col.aggregate:
raise ValueError(
f"groupby[{i}] cannot use aggregate='{col.aggregate}'; "
"aggregates belong in the 'metric' field"
)
```
--
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]
