aminghadersohi commented on code in PR #44744:
URL: https://github.com/apache/superset/pull/44744#discussion_r4123742475
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -767,14 +767,63 @@ def _build_single_query_dict(
return qd
-def _build_gantt_or_big_number_query_dicts(
+def _build_specialized_query_dicts(
form_data: dict[str, Any],
viz_type: str,
metrics: list[Any],
row_limit: int | None,
order_desc: bool | None,
) -> list[dict[str, Any]] | None:
- """Build query dictionaries for the two specialized MCP chart contracts."""
+ """Build query dictionaries matching specialized frontend chart
contracts."""
+ if viz_type == "histogram_v2":
+ column = form_data["column"]
+ groupby = form_data.get("groupby") or []
+ histogram_metrics = (
+ [
+ {
+ "expressionType": "SQL",
+ "sqlExpression": "COUNT(*)",
+ "label": "COUNT(*)",
+ }
+ ]
+ if form_data.get("having")
+ else []
+ )
+ query = _build_single_query_dict(
+ form_data, [*groupby, column], histogram_metrics,
row_limit=row_limit
+ )
+ query["post_processing"] = [
+ {
+ "operation": "histogram",
+ "options": {
+ "column": get_column_name(column),
+ "groupby": [get_column_name(group) for group in groupby],
+ "bins": int(form_data.get("bins", 5)),
+ "normalize": form_data.get("normalize", False),
+ "cumulative": form_data.get("cumulative", False),
+ },
+ }
+ ]
+ return [query]
+
+ if viz_type == "waterfall":
+ axis = form_data.get("x_axis") or form_data.get("granularity_sqla")
+ columns = list(axis) if isinstance(axis, list) else [axis] if axis
else []
+ columns.extend(form_data.get("groupby") or [])
+ query = _build_single_query_dict(
+ form_data, columns, metrics, row_limit=row_limit
+ )
+ query["orderby"] = [(column, True) for column in columns]
+ # Bind the time grain to the SQL time column, as extractExtras does.
+ granularity = form_data.get("granularity",
form_data.get("granularity_sqla"))
+ if granularity is not None:
+ query["granularity"] = granularity
+ if form_data.get("time_grain_sqla") is not None:
+ query.setdefault("extras", {})["time_grain_sqla"] = form_data[
+ "time_grain_sqla"
+ ]
Review Comment:
Reviewed at e7c2e45c2e765e266a98606c3deb9d427be7b525. Not applicable as a
correctness fix: these are not three copies of the same temporal-binding rule.
* `chart_helpers.py:704-716, 769-771`: Treemap binds only granularity,
before shared filter/extras processing. Shared processing applies the top-level
grain and then merges saved extras (`:330-351`).
* `chart_helpers.py:813-824`: Waterfall binds after that shared processing
and explicitly reapplies a non-null top-level grain. Moving this into Treemap's
position would change extras precedence.
* `chart_helpers.py:899-902, 955-960`: deck.gl binds only for the listed
time-series chart types and a truthy grain, sets `is_timeseries`, and reads
`granularity_sqla` directly rather than the normalized-granularity fallback.
The proposed identical-rule path does not occur here.
The small granularity expression is shared text, but a common helper
covering all three would require preserving these different conditions and
ordering; no failing behavioral case is identified by this maintainability
suggestion. Keeping the chart-specific contracts rather than introducing that
refactor in this fix.
Validation: `test_chart_helpers.py` and `test_treemap_completeness.py`: 244
passed, 2 skipped. `pre-commit run --files` on the helper and those two tests:
all applicable hooks passed. No source changes or push.
--
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]
