Skip to content

fix(native-filters): populate granularity_sqla from datasource main_dttm_col for time-range parent cascade #39894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jtjenkins wants to merge 2 commits into
base: master
Choose a base branch
from
+45 −2
Open

fix(native-filters): populate granularity_sqla from datasource main_dttm_col for time-range parent cascade #39894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b50b04
fix(native-filters): populate granularity_sqla from datasource main_d…
jtjenkins May 5, 2026
b582c8a
fix(native-filters): deduplicate in-flight fetchDatasourceMetadata re…
jtjenkins May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion superset-frontend/src/dashboard/actions/datasources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ export function setDatasource(datasource: Datasource, key: string) {
};
}

const inFlightDatasourceKeys = new Set<string>();

export function fetchDatasourceMetadata(key: string) {
return (dispatch: Dispatch, getState: () => RootState) => {
const { datasources } = getState();
Expand All @@ -61,8 +63,21 @@ export function fetchDatasourceMetadata(key: string) {
return dispatch(setDatasource(datasource, key));
}

if (inFlightDatasourceKeys.has(key)) {
return undefined;
}

inFlightDatasourceKeys.add(key);
return SupersetClient.get({
endpoint: `/superset/fetch_datasource_metadata?datasourceKey=${key}`,
}).then(({ json }) => dispatch(setDatasource(json as Datasource, key)));
})
.then(({ json }) => {
inFlightDatasourceKeys.delete(key);
return dispatch(setDatasource(json as Datasource, key));
})
.catch((err: unknown) => {
inFlightDatasourceKeys.delete(key);
throw err;
});
};
}
30 changes: 29 additions & 1 deletion ...-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
onFiltersRefreshSuccess,
setDirectPathToChild,
} from 'src/dashboard/actions/dashboardState';
import { fetchDatasourceMetadata } from 'src/dashboard/actions/datasources';
import {
setHoveredChartCustomization,
unsetHoveredChartCustomization,
Expand Down Expand Up @@ -185,6 +186,28 @@ const FilterValue: FC<FilterValueProps> = ({
const [isRefreshing, setIsRefreshing] = useState(false);
const dispatch = useDispatch();

// When a Date Range parent filter is active, the child filter needs
// granularity_sqla to apply the temporal WHERE clause. Read main_dttm_col
// from the Redux datasource cache as a fallback.
//
// The dashboard datasets API (/api/v1/dashboard/:id/datasets) only returns
// datasets used by chart slices, so native-filter-only datasets are never
// in state.datasources. Dispatch fetchDatasourceMetadata at mount time to
// populate the cache explicitly for those datasets.
const datasourceMainDttmCol = useSelector<RootState, string | null | undefined>(
state =>
datasetId != null
? (state.datasources as Record<string, any>)?.[`${datasetId}__table`]
?.main_dttm_col
: undefined,
);

useEffect(() => {
if (datasetId != null) {
dispatch(fetchDatasourceMetadata(`${datasetId}__table`));
}
}, [datasetId, dispatch]);
Comment on lines +205 to +209
Copy link
Copy Markdown
Contributor

@codeant-ai-for-open-source codeant-ai-for-open-source Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Dispatching metadata fetch on mount without in-flight deduplication can trigger concurrent duplicate requests when multiple filters use the same dataset and mount together. Each instance sees an empty cache and fires its own request, causing avoidable API load and possible last-write-wins state churn; guard this with request deduplication (or a shared loading flag) before dispatching. [race condition]

Severity Level: Minor 🧹 
- ⚠️ Extra GETs to /superset/fetch_datasource_metadata per shared dataset.
- ⚠️ Slightly slower dashboard load for filter-only datasets.
- ⚠️ Unnecessary backend load when many filters share one dataset.
Steps of Reproduction ✅ 
1. Open a dashboard that uses multiple native filters all pointing to the same dataset ID
(each filter's config provides the same `datasetId` in its `targets`), so multiple
`FilterControl` instances render. These controls render `FilterValue` from
`src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx:20-34`,
which in turn mounts multiple `FilterValue` components.

2. When each `FilterValue` mounts, its `useEffect` at `FilterValue.tsx:205-209` runs, and
for each instance with a non-null `datasetId` it dispatches
`fetchDatasourceMetadata(\`${datasetId}__table\`)` via `dispatch`.

3. The thunk `fetchDatasourceMetadata` is defined in
`src/dashboard/actions/datasources.ts:16-28`. It checks `getState().datasources[key]`, and
if the entry is missing it calls `SupersetClient.get({ endpoint:
\`/superset/fetch_datasource_metadata?datasourceKey=${key}\` })` without tracking
in-flight requests.

4. Because each `FilterValue` instance calls `dispatch(fetchDatasourceMetadata(...))` in a
separate effect but before any of the network calls resolve and populate
`state.datasources[key]`, all of them see an empty cache and issue parallel GET requests
to `/superset/fetch_datasource_metadata?datasourceKey=${datasetId}__table`. This results
in duplicate metadata fetches for the same dataset but does not cause functional race
conditions, since each response ultimately dispatches `setDatasource` with the same
payload.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
**Line:** 205:209
**Comment:**
	*Race Condition: Dispatching metadata fetch on mount without in-flight deduplication can trigger concurrent duplicate requests when multiple filters use the same dataset and mount together. Each instance sees an empty cache and fires its own request, causing avoidable API load and possible last-write-wins state churn; guard this with request deduplication (or a shared loading flag) before dispatching.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


const { outlinedFilterId, lastUpdated } = useFilterOutlined();

const handleFilterLoadFinish = useCallback(() => {
Expand Down Expand Up @@ -216,7 +239,11 @@ const FilterValue: FC<FilterValueProps> = ({
groupby,
adhoc_filters: adhocFilters,
time_range: timeRange,
granularity_sqla: granularitySqla,
granularity_sqla:
granularitySqla ||
(dependencies.time_range
? datasourceMainDttmCol ?? undefined
: undefined),
dashboardId,
});
const filterOwnState = filter.dataMask?.ownState || {};
Expand Down Expand Up @@ -347,6 +374,7 @@ const FilterValue: FC<FilterValueProps> = ({
dataMaskSelected,
setHasDepsFilterValue,
transitiveParentIds,
datasourceMainDttmCol,
]);

useEffect(() => {
Expand Down