FrancescoCastaldi commented on code in PR #44464:
URL: https://github.com/apache/superset/pull/44464#discussion_r4112768061
##########
superset/mcp_service/chart/plugins/xy.py:
##########
@@ -103,6 +128,21 @@ def extract_column_refs(self, config: Any) ->
list[ColumnRef]:
if config.filters:
for f in config.filters:
refs.append(ColumnRef(name=f.column))
+ if config.sort_by:
+ sort_entry = config.sort_by
+ if isinstance(sort_entry, list) and sort_entry:
+ sort_entry = sort_entry[0]
+ sort_col = (
+ sort_entry.column
+ if isinstance(sort_entry, SortByConfig)
+ else sort_entry
+ if isinstance(sort_entry, str)
+ else sort_entry.get("column")
+ if isinstance(sort_entry, dict)
+ else None
+ )
+ if sort_col:
+ refs.append(ColumnRef(name=sort_col))
Review Comment:
Thanks for the review and suggestions! This is now fully resolved and
verified across the pipeline as of \ db3ea5\:
- \xtract_column_refs\ checks covered XY names (including x-axis, group-by,
and y-axis metrics/labels and SQL expressions), skipping redundant \ColumnRef\
additions that previously triggered premature validation failures.
- Saved metrics and SQL expression metrics are resolved against
\dataset_context.available_metrics\ with \saved_metric=True\.
- End-to-end integration tests were added in
\TestValidationPipelineWithXYChartSortBy\ to ensure \sort_by\ traverses
\ValidationPipeline\ and sets \ orm_data[\x_axis_sort\]\, correctly resolving
to metric labels (e.g. \SUM(sales)\).
--
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]
