aminghadersohi commented on code in PR #45571:
URL: https://github.com/apache/superset/pull/45571#discussion_r4354740485
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/EchartsSunburst.tsx:
##########
@@ -120,26 +123,53 @@ export default function EchartsSunburst(props:
SunburstTransformedProps) {
const drillToDetailFilters: BinaryQueryObjectFilterClause[] = [];
const drillByFilters: BinaryQueryObjectFilterClause[] = [];
if (columns?.length) {
- treePath.forEach((path, i) =>
- drillToDetailFilters.push({
- col: columns[i],
- op: '==',
- val: records[i],
- formattedVal: path,
- }),
- );
- const val = treePath[treePath.length - 1];
- drillByFilters.push({
- col: columns[treePath.length - 1],
- op: '==',
- val,
- formattedVal: formatSeriesName(val, {
- timeFormatter: getTimeFormatter(formData.dateFormat),
- numberFormatter: getNumberFormatter(formData.numberFormat),
- coltype:
- coltypeMapping?.[getColumnLabel(columns[treePath.length - 1])],
- }),
+ const timeGrain = (formData.extraFormData?.time_grain_sqla ??
+ formData.timeGrainSqla) as TimeGranularity | undefined;
+ treePath.forEach((path, i) => {
+ // Range filters apply the dataset timezone/offset to both bucket
+ // bounds. Equality on a truncated column bypasses that handling.
+ const temporalFilter =
+ timeGrain &&
+ isGrainedLevel(
+ columns[i],
+ formData.temporalColumnsLookup,
+ formData.granularitySqla,
+ )
+ ? getTemporalXAxisDrillByFilter(
Review Comment:
I corrected the contradictory documentation in
296b1fc0b2792aa9d2da9f174b95eca67747df53. When a Time Column override replaces
a different temporal column, Sunburst retains independent equality/membership
predicates on that hierarchy level; other chart types, including Table and
time-series, discard predicates on the replaced column. This matches
`_apply_granularity` and the preceding Sunburst paragraph. This commit changes
only documentation, so no new regression test is needed for this part.
Regression test for the code change in
c51044bac6f96fa85048c77c5f13eb44686bbf34:
`tests/unit_tests/models/test_sunburst_drill_hours_offset.py::test_sunburst_factory_drill_keeps_bucket_on_different_time_column`,
added by this PR. I reran it after temporarily restoring the pre-fix
`_apply_granularity` condition: `pytest ... -k
factory_drill_keeps_bucket_on_different_time_column -q` produced `6 failed, 42
deselected`, with samples returning two extra rows (6 instead of 4). With the
fix, `pytest tests/unit_tests/models/test_sunburst_drill_hours_offset.py -k
'factory_drill_keeps_bucket_on_different_time_column or
timeseries_samples_with_swapped_time_column_drops_stale_drill_filter or
non_sunburst_granularity_preserves_independent_filters' -q -m 'not manual and
not llm_compliance'` produced `26 passed, 22 deselected`. The temporary source
change was restored; `git status --short` showed only the staged docs change
and untracked scratch files before committing.
`pre-commit run --files docs/docs/using-superset/exploring-data.mdx` exited
0 (applicable hooks passed).
Siblings checked:
- docs/docs/using-superset/exploring-data.mdx:503 — fixed in
296b1fc0b2792aa9d2da9f174b95eca67747df53
- docs/docs/using-superset/exploring-data.mdx:491 — not affected: already
states that the Sunburst bucket remains on its hierarchy level under a
different Time Column.
- docs/docs/using-superset/creating-your-first-dashboard.mdx:217 — not
affected: describes semantic-view date bounds, not replacement of a chart's
Time Column.
- superset/common/query_context_factory.py — not affected: retains
non-TEMPORAL_RANGE predicates when the Time Column is unchanged or the chart is
Sunburst; the existing row regression and Table/time-series checks passed.
I searched the docs with `rg -n 'replaced column|filters.*discarded|Time
Column override' docs --glob '*.md*'` and `rg -n 'Time Column|replaced
column|independent.*filter' docs --glob '*.md*'` and inspected the matching
paragraphs.
--
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]
