aminghadersohi commented on code in PR #44105:
URL: https://github.com/apache/superset/pull/44105#discussion_r3975333947
##########
tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py:
##########
@@ -1406,3 +1408,75 @@ def capture_create(**kwargs):
assert since is not None
assert until is not None
assert since < until
+
+
[email protected](
+ ("expression", "expected_start", "expected_end"),
+ [
+ ("Last month", "2026-06-17T00:00:00", "2026-07-17T00:00:00"),
+ ("Last year", "2025-07-17T00:00:00", "2026-07-17T00:00:00"),
+ ("previous calendar month", "2026-06-01T00:00:00",
"2026-07-01T00:00:00"),
+ ("Current year", "2026-01-01T00:00:00", "2027-01-01T00:00:00"),
+ ("2025-06-01 : 2025-07-01", "2025-06-01T00:00:00",
"2025-07-01T00:00:00"),
+ ("No filter", None, None),
+ ],
+)
[email protected]("use_filter", [False, True])
[email protected]("result_kind", ["fresh", "cached", "empty"])
[email protected]
+async def test_query_dataset_returns_engine_time_bounds(
+ mcp_server: FastMCP,
+ expression: str,
+ expected_start: str | None,
+ expected_end: str | None,
+ use_filter: bool,
+ result_kind: str,
+) -> None:
+ """Resolve MCP inputs with the real factory and serialize execution
bounds."""
+ from flask import current_app
+ from freezegun import freeze_time
+
+ from superset.common.chart_data import ChartDataResultType
+ from superset.common.query_object_factory import QueryObjectFactory
+
+ dataset = _make_dataset(main_dttm_col="order_date")
+
+ def execute(
+ datasource_id: int, datasource_type: str, query_dict: dict[str, Any],
**_: Any
+ ) -> dict[str, Any]:
+ """Use production date resolution in place of database execution."""
+ factory = QueryObjectFactory(current_app.config, MagicMock())
+ with freeze_time("2026-07-17 12:34:56"):
+ query = factory.create(
+ parent_result_type=ChartDataResultType.FULL,
+ **query_dict,
+ )
+ payload = _mock_command_result()
+ result = payload["queries"][0]
+ result.update(from_dttm=query.from_dttm, to_dttm=query.to_dttm)
+ result["is_cached"] = result_kind == "cached"
Review Comment:
Addressed with `test_query_dataset_cached_bounds_across_rollover`, covering
both populated and empty results. It uses the real `QueryObjectFactory`,
`QueryContextProcessor.get_df_payload`, and `QueryCacheManager` with a
SimpleCache backend, so the first request serializes the cache entry and the
second reconstructs it. The clock crosses midnight within the cache TTL;
assertions verify identical cache keys, only one source execution, preserved
cached rows, a real cache hit, and the new request's resolved bounds in the MCP
JSON response.
One important distinction from the suggested fix: the cache entry does not
store `from_dttm`/`to_dttm`. Production obtains them from the current
QueryObject when assembling the result, rather than restoring the original
execution's bounds. The regression test explicitly verifies that behavior
instead of seeding artificial cached bounds.
##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -270,6 +272,8 @@ def _build_response(
)
return GetTableResponse(
+ from_dttm=query_result.get("from_dttm"),
+ to_dttm=query_result.get("to_dttm"),
Review Comment:
Checked this against the MCP entry point. The shared query builder supports
that rewrite, but `GetTableRequest` validation rejects open-ended strings such
as `"2025-01-01 : "` and `" : 2025-02-01"` before they reach execution.
`validate_time_range` strips the input, which removes the space needed for the
`" : "` separator, and the remaining string fails bounded-range validation.
Added `test_get_table_rejects_open_ended_range_before_execution` for both
directions and both dataset/view selectors. It calls the MCP tool and asserts
the validation error and that `execute_tabular_query` is never called. Updated
the documentation to distinguish this from the shared builder: callers can use
explicit comparison filters for one-sided constraints, but those are not engine
primary bounds. No synthetic bounds or query behavior changes are needed for
this PR.
--
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]
