codeant-ai-for-open-source[bot] commented on code in PR #39894:
URL: https://github.com/apache/superset/pull/39894#discussion_r3190556597
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx:
##########
@@ -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]);
Review Comment:
**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]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ 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.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
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.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterControls%2FFilterValue.tsx%0A%2A%2ALine%3A%2A%2A%20205%3A209%0A%2A%2AComment%3A%2A%2A%0A%09%2ARace%20Condition%3A%20Dispatching%20metadata%20fetch%20on%20mount%20without%20in-flight%20deduplication%20can%20trigger%20concurrent%20duplicate%20requests%20when%20multiple%20filters%20use%20the%20same%20dataset%20and%20mount%20together.%20Each%20instance%20sees%20an%20empty%20cache%20and%20fires%20its%20own%20request%2C%20causing%20avoidable%20API%20load%20and%20possible%20last-write-wins%20state%20churn%3B%20guard%20this%20with%20request%20deduplication%20%28or%20a%20shared%20loading%20flag%29%20before%20dispatching.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20pr
opose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FnativeFilters%2FFilterBar%2FFilterControls%2FFilterValue.tsx%0A%2A%2ALine%3A%2A%2A%20205%3A209%0A%2A%2AComment%3A%2A%2A%0A%09%2ARace%20Condition%3A%20Dispatching%20metadata%20fetch%20on%20mount%20without%20in-flight%20deduplication%20can%20trigger%20concurrent%20duplicate%20requests%20when%20multiple%20filters%20use%20the%20same%20dataset%20and%20mount%20together.%2
0Each%20instance%20sees%20an%20empty%20cache%20and%20fires%20its%20own%20request%2C%20causing%20avoidable%20API%20load%20and%20possible%20last-write-wins%20state%20churn%3B%20guard%20this%20with%20request%20deduplication%20%28or%20a%20shared%20loading%20flag%29%20before%20dispatching.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
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
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39894&comment_hash=3b8f518c4b11c88d04c2b181f8e8201a176dc6d44790837937cabfe8d8bd31ab&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39894&comment_hash=3b8f518c4b11c88d04c2b181f8e8201a176dc6d44790837937cabfe8d8bd31ab&reaction=dislike'>๐</a>
--
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]
