sadpandajoe commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4225950175
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1398,6 +2413,137 @@ def fallback_vega_lite_preview(
return None
+def _xy_pivot_x_type(values: list[Any]) -> str:
+ """Infer the Vega-Lite x type from every x value, not a character scan.
+
+ The renderer has no column metadata, so text is temporal only when each
+ value parses as an ISO date or datetime; labels such as ``New York`` stay
+ nominal instead of becoming unparseable dates.
+ """
+ present = [value for value in values if value is not None]
+ if not present:
+ return "nominal"
+ if all(
+ isinstance(value, (int, float)) and not isinstance(value, bool)
+ for value in present
+ ):
+ return "quantitative"
+ if all(
+ isinstance(value, (date, datetime))
+ or (isinstance(value, str) and _gantt_temporal_value(value) is not
None)
Review Comment:
A grouped XY result whose x values are compact ISO dates (`20250101`,
`20250102`) passes this check, because Python's ISO parser accepts that form,
so the preview marks x as `temporal` and leaves the strings unchanged. Vega
parses temporal strings with `Date.parse`, which returns `NaN` for them, so the
bars drop out of an otherwise successful preview. Should the inference only
accept the extended ISO forms the renderer can parse, and fall back to
`nominal` otherwise?
##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -509,33 +563,38 @@ async def _run_get_table_query(
use_cache=request.use_cache,
force=request.force_refresh,
)
+ extracted = _extract_table_query_result(result)
+ if isinstance(extracted, SemanticLayerError):
+ return extracted
+ query_result, data, raw_columns, coltypes = extracted
query_duration_ms = int((time.time() - start_time) * 1000)
- if not result or "queries" not in result or not result["queries"]:
- return SemanticLayerError.create(
- error="Query returned no results.",
- error_type="EmptyQuery",
- )
-
await ctx.report_progress(5, 5, "Formatting results")
- query_result = result["queries"][0]
response = _build_response(
request,
is_builtin,
resolved.display_name,
query_result,
+ data,
+ raw_columns,
+ coltypes,
query_duration_ms,
resolved.warnings,
resolved.temporal_columns,
resolved.valid_grains,
)
+ if response_failure := response_json_failure(response):
Review Comment:
Nothing exercises this post-format `response_json_failure` guard through
`get_table`. The existing malformed-result cases
(`test_get_table_maps_invalid_result_without_formatting_hooks`) all reject at
source validation, before formatting runs, so deleting this block would leave
them passing. Could you add a case with about 255 rows that each carry a
distinct 64 KiB `category` cell (valid at the source) and assert that the
repeated profiling samples produce a `MalformedQueryResult` error with no data
returned?
--
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]
