aminghadersohi commented on code in PR #44146:
URL: https://github.com/apache/superset/pull/44146#discussion_r4051259435


##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -1180,3 +1180,73 @@ once, even when multiple dimensions are missing.
 The tool does not rewrite dots into SQL path separators: quoting and 
nested-field
 support depend on the dataset's database dialect, and dots can also be literal
 characters in column names.
+
+## Dataset discovery and routing
+
+`list_datasets` searches table name, **description**, schema, and SQL using
+escaped, case-insensitive substring matching. Write descriptions with business
+terminology to make differently named datasets discoverable. A complete UUID
+passed as `search` is treated as an exact UUID filter; an explicit `uuid` 
filter
+can also look up one or more datasets by UUID. Matching
+datasets are candidates, ordered by the requested sort (last modified by 
default), not
+ranked recommendations. Compare descriptions and metrics, present alternatives
+when ambiguous, and clarify before querying. No matches does not prove that 
data
+does not exist; search only covers accessible datasets and the configured 
scope.
+
+`query_dataset` returns `dataset_id` and `dataset_name`; `get_table` 
additionally
+returns `source` and the corresponding dataset or external-view identity. Cite
+these fields in answers rather than guessing the source from the query text.
+Neither tool selects an alternative dataset automatically.
+
+### Optional per-role dataset scope
+
+Set `MCP_DATASET_ROLE_ALLOWLIST` in `superset_config.py` to constrain MCP to a
+curated set of registered datasets:
+
+```python
+MCP_DATASET_ROLE_ALLOWLIST = {
+    "Finance Readers": ["00000000-0000-0000-0000-000000000001"],
+    "Operations Readers": ["00000000-0000-0000-0000-000000000002"],
+}
+```
+
+Obtain dataset UUIDs from `get_dataset_info` (or request `uuid` in
+`list_datasets.select_columns`). Use UUIDs, not names or numeric IDs, in 
config.
+Effective roles, including group roles, contribute the **union** of their 
lists.
+That union is **intersected with existing dataset access**. Unconfigured roles
+contribute nothing; Admin has no routing exemption. `None` (the default) 
disables
+this feature; `{}` allows no datasets. Invalid entries cause a configuration
+error at startup rather than silently disabling the restriction.
+
+Scoped mode supports the dataset tools that name a registered dataset —
+`list_datasets`, `get_dataset_info`, `query_dataset`, and, with a built-in
+dataset, `get_table`, `list_metrics`, `get_compatible_dimensions` and
+`get_compatible_metrics` — plus `health_check` and `get_schema`.
+Discovery filters before counting and pagination. Every other **authenticated**
+tool refuses in this mode, including SQL Lab, chart/dashboard previews and 
data,
+cached query results, external semantic sources, mutations, and extension 
tools.
+The one exception is `generate_bug_report`, which is deliberately public
+(`protect=False`, listed in `ALLOWED_UNPROTECTED`) so diagnostics stay 
available
+even when authentication is broken. Scoped mode is enforced in the
+authentication wrapper, so that tool keeps working; it reports environment and
+user-supplied diagnostics and reads no dataset rows. These paths cannot
+all be attributed reliably to registered dataset UUIDs; scoped mode 
deliberately
+refuses them instead of attempting SQL lineage inference. Unsupported tools 
stay
+listed and refuse when called, so the assistant reads an explanation it can 
pass
+on rather than finding the tool missing. Prompts and resources are unaffected:
+they expose schema and instance metadata, not dataset rows. This restriction
+applies to every caller of the MCP tools, not just a particular chat client.
+
+An out-of-scope request returns an explicit refusal. The assistant must explain
+that limitation, never silently substitute an allowed but different dataset.
+This feature controls routing and usability, **not authorization or SQL table
+access**. Existing dataset permissions, query validation, and row-level 
security
+remain mandatory and unchanged. A registered virtual dataset may itself query
+multiple physical tables; this configuration selects registered dataset 
objects,
+not their underlying SQL dependencies. It does not restrict the Superset 
UI/API.
+
+Description search adds one text predicate to an existing catalog substring
+scan, without joins or additional queries, and replaces the `uuid` predicate
+that could never match. It is not an indexed full-text search: large catalogs

Review Comment:
   Applied your suggestion verbatim in 983158a — thanks, the claim was wrong 
and you traced it to the right place.
   
   You're right that `d7f0efc` is the counterexample: the cast-to-text 
predicate did match a complete UUID on PostgreSQL and CockroachDB, which is 
exactly why `list_datasets` keeps that path working via the exact-`uuid` 
filter. "Varied by database" now matches the comment on 
`DATASET_SEARCH_COLUMNS` in `schema_discovery.py`, so the docs and the code say 
the same thing.
   
   Grepped the rest of the docs and the MCP service for other survivors of the 
"never match" phrasing. The only remaining one is in `dataset_scope.py`, about 
an allowlist entry that is a syntactically valid UUID no dataset actually has — 
a different claim, and still accurate.



-- 
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]

Reply via email to