bito-code-review[bot] commented on PR #43573:
URL: https://github.com/apache/superset/pull/43573#issuecomment-5438908712

   <!-- Bito Reply -->
   The issue is correct. The `SankeyChartConfig` validator currently only 
checks for `sql_expression` and `saved_metric` on `source` and `target` 
dimensions, but it fails to reject the `aggregate` field, which leads to silent 
semantic changes in the generated Sankey configuration. To resolve this, update 
the `reject_metric_style_nodes` validator in 
`superset/mcp_service/chart/schemas.py` to also check for and reject the 
`aggregate` field on `source` and `target` columns.
   
   **superset/mcp_service/chart/schemas.py**
   ```
   @model_validator(mode="after")
       def reject_metric_style_nodes(self) -> "SankeyChartConfig":
           """source and target are node dimensions, not metrics."""
           for col, name in ((self.source, "source"), (self.target, "target")):
               _reject_sql_expression_on_dimension(col, name)
               if col and (col.saved_metric or col.aggregate):
                   raise ValueError(
                       f"{name} cannot use saved_metric=True or aggregate; "
                       "saved metrics and aggregates belong in the 'metric' 
field"
                   )
           return self
   ```


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