codeant-ai-for-open-source[bot] commented on code in PR #41281:
URL: https://github.com/apache/superset/pull/41281#discussion_r3599109827
##########
superset-frontend/src/SqlLab/reducers/sqlLab.ts:
##########
@@ -729,7 +729,13 @@ export default function sqlLabReducer(
extra_json: JSON.parse(db.extra || ''),
};
});
- return { ...state, databases };
+ return {
+ ...state,
+ databases: {
+ ...state.databases,
+ ...databases,
+ },
Review Comment:
**Suggestion:** Unconditionally merging incoming databases with existing
Redux state turns `SET_DATABASES` into an append-only cache, so databases that
are removed from later server responses (for example after permission/exposure
changes or a fresh first-page reload) are never evicted and can remain
selectable as stale entries. Preserve merge behavior only for pagination/“load
more” paths, and replace state on fresh loads (e.g., first page or new search)
so revoked/removed databases are dropped. [cache]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SQL Lab database cache retains revoked/hidden database metadata.
- ⚠️ QueryAutoRefresh uses stale allow_run_async from old databases.
- ⚠️ useDatabaseSelector may show metadata for non-exposed databases.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open SQL Lab, which renders `QueryAutoRefresh` from
`src/SqlLab/components/App/index.tsx:15-18`, and consumes `sqlLab.databases`
from Redux as
seen in `src/SqlLab/components/QueryAutoRefresh/index.tsx:7-13`.
2. Use the DB selector popover in the left bar, which renders
`DatabaseSelector` with
`getDbList={dbSelectorProps.getDbList}` at
`src/SqlLab/components/SqlEditorLeftBar/index.tsx:14-19`, wired through
`useDatabaseSelector`
(`src/SqlLab/components/SqlEditorTopBar/useDatabaseSelector.ts:40-53`) where
`getDbList`
dispatches `setDatabases` (`src/SqlLab/actions/sqlLab.ts:612-14`).
3. Trigger a database search or page load in `DatabaseSelector` so its
`loadDatabases`
function (`src/components/DatabaseSelector/index.tsx:216-33`) calls
`/api/v1/database`
with specific `search`, `page`, and `page_size` filters and then invokes
`getDbList(result)` with only the current page’s `result` array.
4. Later, have the backend stop returning one previously loaded database
(e.g.,
`expose_in_sqllab` toggled off or permissions changed) so a fresh first-page
search
returns a `result` lacking that DB; observe that the reducer’s
`SET_DATABASES` handler
(`src/SqlLab/reducers/sqlLab.ts:23-39`) still merges new `databases` into
`state.databases` instead of replacing it, leaving the old DB entry cached
and available
to consumers like `QueryAutoRefresh` and `useDatabaseSelector` even though
the server
intentionally omitted it.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=374bcf0f393b47f3ae558ef4d5dd9ac6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=374bcf0f393b47f3ae558ef4d5dd9ac6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(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/SqlLab/reducers/sqlLab.ts
**Line:** 734:737
**Comment:**
*Cache: Unconditionally merging incoming databases with existing Redux
state turns `SET_DATABASES` into an append-only cache, so databases that are
removed from later server responses (for example after permission/exposure
changes or a fresh first-page reload) are never evicted and can remain
selectable as stale entries. Preserve merge behavior only for pagination/“load
more” paths, and replace state on fresh loads (e.g., first page or new search)
so revoked/removed databases are dropped.
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%2F41281&comment_hash=b5daa18c84ed37544135e4a45733b839361451498f022139486471c8761afa18&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41281&comment_hash=b5daa18c84ed37544135e4a45733b839361451498f022139486471c8761afa18&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]
