mikebridge commented on code in PR #43781:
URL: https://github.com/apache/superset/pull/43781#discussion_r3935847208
##########
superset/models/slice.py:
##########
@@ -230,6 +231,59 @@ def _display_datasource(self) -> SqlaTable | SemanticView
| None:
return self.semantic_view
return self.table
+ @property
+ def resolved_datasource(self) -> Datasource | None:
+ """The chart's datasource, resolved across datasource types.
+
+ ``Slice.datasource`` is pinned to table-backed datasources (the
+ ``table`` relationship joins on ``datasource_type == 'table'``), so
+ charts on other datasource types — semantic views in particular —
+ resolve to ``None`` there. Authorization call sites must use this
+ resolver instead, so those charts participate in access checks
+ rather than silently vanishing from them.
+
+ Returns ``None`` when the datasource row does not exist, the type is
+ unknown, or the resolved model does not participate in access
+ control (no ``perm``, e.g. ``SavedQuery``); callers must treat
+ ``None`` as inaccessible, never as absent. Non-table lookups issue a
+ database query on every access — deduplicate before calling this in
+ a loop.
+ """
+ if not self.datasource_id:
+ return None
+ if self.datasource_type == utils.DatasourceType.TABLE:
+ return self.table
+ if self.datasource_type == utils.DatasourceType.SEMANTIC_VIEW:
+ # Resolved through the type-guarded ``semantic_view`` relationship
+ # rather than a DAO query: identity-map cached, and its join
+ # predicate already enforces the type constraint. ``None`` when
+ # the row is gone, matching the DAO fallback's semantics.
+ return self.semantic_view
+ # pylint: disable=import-outside-toplevel
+ # Deferred to avoid a circular import: superset.daos.datasource
+ # imports connectors and sql_lab models at module top.
+ from superset.daos.datasource import DatasourceDAO
+ from superset.daos.exceptions import (
+ DatasourceNotFound,
+ DatasourceTypeNotSupportedError,
+ DatasourceValueIsIncorrect,
+ )
+
+ try:
+ resolved = DatasourceDAO.get_datasource(
+ self.datasource_type, self.datasource_id
+ )
Review Comment:
Assessed — real but bounded, deferred with a ticket. Only the DAO fallback
issues queries: table and semantic-view charts (the overwhelming majority)
resolve through identity-map-cached relationships with zero extra queries, so
the N+1 applies solely to other perm-carrying types — in practice `query`-typed
charts, a rare legacy path. The dashboard gate already bounds it: membership
evaluation dedupes by `(datasource_type, datasource_id)` and short-circuits on
the first accessible member, and the docstring warns against un-deduplicated
loop use. Batching the residue means a grouped per-type `IN` fetch through
`DatasourceDAO` — a surface change out of proportion for this authorization
fix, so it's filed as SC-119903 (epic: Semantic Layers foundation, related to
this PR's story) with the batching shape recorded. Resolving as
deferred-to-SC-119903.
--
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]
