tomerkl65 commented on issue #43331: URL: https://github.com/apache/superset/issues/43331#issuecomment-5348927896
Appreciate everyone weighing in. Let me answer the four questions grounded in how the PR actually works today. **Where the current implementation sits** Right now the hierarchy lives in `form_data` as `drilldown_hierarchy: string[]` (an ordered list of column names), configured per chart via a reused drag-and-drop column control. It's a pure interaction layer: it does **not** touch the base/undrilled query, it only defines what happens on click (swap dimension + add a filter for the clicked value, with a breadcrumb to step back). So it's additive and low-risk for existing charts. That means today it *is* the "form data / chart's responsibility" model — with all the tradeoffs you called out. **On the four questions** 1. **Form data / per-chart responsibility.** Agreed this is the main drawback: to reuse the same hierarchy across N charts you redefine it N times. It's the pragmatic v1 because it needs zero schema/backend changes and works uniformly across every ECharts chart, but it doesn't scale to "define once, reuse everywhere." 2. **Brittleness to data-model changes.** Correct — since we store raw column names, a column rename/removal silently breaks the hierarchy (same failure mode as any column reference in form data today, e.g. groupby/x-axis). Storing a *reference* to a named hierarchy rather than the raw list would localize that breakage to one place. 3. **Dataset-level definition.** I think this is the right long-term home for *reuse*. Concretely: add a "hierarchies" concept to the dataset (a sibling to columns/metrics) — a named, ordered list of columns. Charts then store a reference (`hierarchy: "geo"`) instead of the inline list. Benefits: define once, reuse across charts, survive column changes in one place, and it becomes discoverable in the Explore UI. The interaction/breadcrumb layer we already built is agnostic to *where* the list comes from, so this is an evolution, not a rewrite. 4. **Semantic layers.** If a dataset is backed by a semantic layer, that's where hierarchies should *really* be authored, and Superset should sync/import them rather than have users re-enter them. That argues for making the dataset-level hierarchy an *interface* that can be populated either manually or from an upstream semantic layer — not a Superset-only construct. So the dataset model in (3) should be designed to be "hydrated from" a semantic layer, not competing with it. **Suggested path** Ship the chart-level version now as the interaction foundation (guided, predefined drill path — complementary to Drill By's ad-hoc/global drilling, not a replacement). Treat dataset-level named hierarchies (hydratable from semantic layers) as the next SIP iteration, where the chart just references a hierarchy by name. Nothing in the click/breadcrumb layer needs to change when we do. **On the feature flag** Totally aligned on not proliferating flags. It's currently dark-launched behind `DRILL_DOWN` purely so it isn't exposed mid-review. If there's SIP consensus, I'm happy to drop the flag and ship it un-gated. (If folks want a safety valve, keep it for one release for easy rollback and remove it right after — but I don't feel strongly.) -- 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]
