geido commented on code in PR #43719:
URL: https://github.com/apache/superset/pull/43719#discussion_r3915017830
##########
superset/datasets/api.py:
##########
@@ -1757,6 +1757,8 @@ def get_drill_info(self, pk: int, **kwargs: Any) ->
Response:
"columns.column_name",
"columns.verbose_name",
"columns.groupby",
+ "metrics.metric_name",
Review Comment:
Fair to want this stated rather than assumed. The explicit answer:
- **Size.** Two short strings per metric. Datasets carry far fewer metrics
than columns, and `columns` already ships `column_name` + `verbose_name` for
every `groupby=True` column, so `metrics` is a small fraction on top of what
the response already sends.
- **Request count.** `useDatasetDrillInfo` goes through `cachedSupersetGet`,
keyed on the endpoint. N charts on the same dataset produce one request per
dashboard load, not N — so this is one slightly larger response per dataset,
not per chart.
- **Why `columns` is narrowed.** The `post_dump` filter isn't primarily a
payload-size measure, it's semantic: the drill-by picker only accepts
dimensions, so non-dimension columns are noise there. Metrics have no
equivalent "not applicable" subset for label resolution — the grid needs a
label for whichever metric a chart references.
- **The alternative is worse here.** Scoping to the metrics a given chart
uses (the other thread's suggestion) would key the response per chart rather
than per dataset, multiplying requests per dashboard load. So this is a
deliberate trade: one slightly larger shared, cached response instead of many
small per-chart ones.
Worth noting this PR also *increases* who fetches `drill_info`, per the two
comments below — a `can_view_chart_as_table` user now fetches it where they
previously did not. That is the point of the fix, and the caching above is what
keeps it to one request per dataset.
--
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]
