rebenitez1802 commented on PR #43829:
URL: https://github.com/apache/superset/pull/43829#issuecomment-5588247807

   Thanks Mike — and good catch on the asymmetry. **Confirming it's intended**, 
and I've documented the single rule in `_resolve_source_type`'s docstring 
(`2ec3f40`) so it reads as deliberate rather than surprising.
   
   The three paths follow one precedence order (highest → lowest):
   
   1. **Access** — a principal never sees a source type it can't read; a 
dataset-only filter (schema/sql) from a user without dataset access yields 
`empty`.
   2. **Explicit `Source` selection** — authoritative; it suppresses 
otherwise-contradictory cross-type filters (a leftover Schema chip becomes a 
no-op rather than a contradiction). This is the *one* place a dataset-only 
filter is intentionally dropped instead of yielding `empty` — the settled 
"explicit selection wins" rule (endorsed by @aminghadersohi, pinned by 
`test_resolve_source_type_explicit_semantic_layer_wins_over_schema`).
   3. **Implicit narrowing + content filters** — honest-AND: a filter that 
can't match the resulting rows returns `empty` rather than being dropped. 
Covers `Type="Semantic View"` + schema and the semantic-layer-*connection* + 
schema route.
   
   So your table is exactly the intended behavior: the explicit 
`Source="Semantic layer"` selection is the deliberate escape hatch (rule 2), 
while the Type-filter and connection routes are honest-AND (rule 3). The 
permission-dependency is rule 1 outranking rule 2 — a user who can't read 
datasets can't get schema-filtered results regardless of source selection. 
Aligning the explicit path to `empty` would reverse the settled call with Amin, 
so I kept it and documented the precedence instead.
   
   On the optional follow-ups: agreed a `ParsedFilters` NamedTuple would be 
cleaner than the 7-tuple — I'd rather do that as its own small refactor (your 
"next growth ring") than grow this diff further. And noted on 
`_apply_sql_null_filter` — it's behaviour-preserving (mutation-checked), happy 
to leave it.
   
   Let me know if you'd like the precedence rule surfaced anywhere beyond the 
docstring.


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