gabotorresruiz commented on code in PR #44893:
URL: https://github.com/apache/superset/pull/44893#discussion_r4187149721
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -2489,33 +2553,78 @@ class ApplyFilterValueSpec(BaseModel):
"'No filter' to clear the filter."
),
)
+ range: List[int | float | None] | None = Field(
Review Comment:
Small symmetry question with `_reject_bool_dataset_id`: `bool` is a subclass
of `int` here too, so `range: [true, 100]` is accepted and lands as a real
predicate. I ran it through the tool on this head and got `{"col": "cost",
"op": ">=", "val": 1}` in the mask with `range: [1, 100]` in the summary and no
error, and `false` gives `>= 0` the same way. Worth hanging the same
before-validator on the bounds, or is a caller passing a boolean bound not
worth guarding against?
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -1658,3 +1666,18 @@ predicate, whose behavior varied by database. It is not
an indexed
full-text search: large catalogs and long descriptions can increase scan time.
Benchmark with your metadata database and catalog size when tuning discovery
latency.
+
+### Managing and applying dashboard native filters
+
+`manage_native_filters` requires a numeric dataset column when adding a
+`filter_range` filter or changing its dataset/column target. Select filters
+continue to accept nonnumeric columns.
+
+For `apply_dashboard_filters`, pass `time_grain` as a list containing at most
+one datasource-supported duration (for example, `["P1D"]`). Custom engine
+durations are accepted when exposed in the datasource's time-grain options,
+rather than limited to a fixed enum.
+A nonempty `time_grains` list in the saved filter configuration restricts the
+allowed selection and takes precedence over dataset options; missing or empty
Review Comment:
Not a blocker, but this sentence now describes the behaviour from before the
allowlist fix. `_timegrain_data_mask` intersects the two sets instead of
letting the allowlist win: the allowlist check at
`apply_dashboard_filters.py:344` runs first, and the dataset check at `:371`
now runs unconditionally right after it, so an allowlisted grain the dataset
does not support is rejected. I verified it both ways: `time_grains: ["PT6H"]`
against a Postgres dataset is rejected on this head and accepted on `95fd12b`,
which is exactly what
`test_timegrain_in_allowlist_but_unsupported_by_dataset_is_rejected` pins. The
paragraph at line 120 reads the same way. Maybe "must appear in both the
allowlist and the dataset's own grains"?
--
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]
