mikebridge commented on PR #45108: URL: https://github.com/apache/superset/pull/45108#issuecomment-6070892588
@aminghadersohi @rebenitez1802 — could I get your eyes on this when you have a moment? It is documentation plus one behavior-preserving extraction, so the interesting part is whether the rules as written match what we actually want providers to rely on. Two spots in particular: 1. **Catalog vs query uniqueness** in the `Metric`/`Dimension` and `SemanticView` docstrings. An earlier draft of this said flatly that a view must not expose two members sharing a name, which contradicts the supported grain-variant shape two paragraphs above it — a provider following that would have deleted valid temporal variants. It now separates "dimensions unique per `(name, grain)`" from "one selected variant, and so one result column, per name". Worth checking that split reads the way you expect. 2. **The two forward references.** The grain preference is #44454's rule and the NULL split is host work still in flight, so both docstrings say so rather than claiming the host behaves that way today. If either lands with a different rule, these docstrings need updating with it. `members_by_key` is a straight extraction of the existing comprehension — `test_matches_the_lookup_it_replaces` asserts the same mapping and the same key order, and it takes an iterable rather than the view so the provider is still queried once per call. Kept as a draft until reviewed. -- 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]
