aminghadersohi commented on code in PR #42144:
URL: https://github.com/apache/superset/pull/42144#discussion_r3616180980
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -807,6 +807,22 @@ class QueryDatasetFilter(BaseModel):
)
+# Bracket shorthands (e.g. "[year]", "[quarter]") are not a Superset
+# time-range grammar — they appear when an LLM copies a grain token from a
+# dashboard filter context. Map them to the equivalent "Last <unit>" form
+# that get_since_until() does support.
+_BRACKET_SHORTHAND_TO_TIME_RANGE: dict[str, str] = {
+ "[second]": "Last second",
+ "[minute]": "Last minute",
+ "[hour]": "Last hour",
Review Comment:
Good catch — confirmed `hour` is missing from the `this|last|next|prior`
unit alternation in `get_since_until()`'s `time_range_lookup` regex
(`superset/utils/date_parser.py`). However, fixing that regex alone isn't
sufficient: even "Last minute"/"Last second", which already matched the
existing regex, fail end-to-end with a *different* pre-existing bug — the
since-expression resolves against `now` while the default until-expression
resolves against `today` (midnight), so `since > until" and `get_since_until`
raises "From date cannot be larger than to date".
Rather than patching the shared `date_parser.py` regex (used everywhere in
Superset, out of scope for this PR), I mapped `[hour]`/`[minute]`/`[second]` to
explicit `DATEADD(DATETIME('now'), -1, <UNIT>) : DATETIME('now')` expressions
in `schemas.py`, which resolve both ends against `now` and avoid the mismatch
entirely. Added regression tests for all three. See be369eed50.
--
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]
