codeant-ai-for-open-source[bot] commented on code in PR #42203:
URL: https://github.com/apache/superset/pull/42203#discussion_r3608701979
##########
superset/db_engine_specs/base.py:
##########
@@ -1236,6 +1236,7 @@ def fetch_data(cls, cursor: Any, limit: int | None =
None) -> list[tuple[Any, ..
)
}
if column_mutators:
+ data = list(data)
Review Comment:
**Suggestion:** Unconditionally copying `data` to a new list when mutators
are present duplicates the full result set in memory even when `fetchall()`
already returned a mutable list. On large result sets this can cause avoidable
memory spikes and potential worker OOMs; only copy when the returned container
is immutable (for example, a tuple) so mutable list results can be mutated in
place. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Large Postgres queries may double memory, risking OOM.
- ⚠️ SQL Lab queries (`sql_lab.py:303`) duplicate result lists.
- ⚠️ Datasource introspection (`sqla/utils.py:162`) duplicates result
containers.
- ⚠️ Chart data fetching (`models/core.py:865`) uses fetch_data.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Postgres database using psycopg2 and a table with an INTERVAL
column;
Superset uses `PostgresEngineSpec.column_type_mutators` for INTERVAL (see
`superset/db_engine_specs/postgres.py:26-28`).
2. In SQL Lab, run a query returning many rows including that INTERVAL
column; the
execution path calls `db_engine_spec.fetch_data(cursor, increased_limit)` at
`superset/sql_lab.py:303`.
3. Inside `BaseEngineSpec.fetch_data`
(`superset/db_engine_specs/base.py:13-49`),
`cursor.fetchall()` returns a list of row tuples for psycopg2, and
`column_mutators` is
built; because an INTERVAL column is present, `column_mutators` is non-empty
and the
mutator branch is taken.
4. The mutator branch executes `data = list(data)` at
`superset/db_engine_specs/base.py:39-41`, creating a second full list copy
even though
`data` is already a list; for large result sets this doubles in-memory row
storage
temporarily, which can cause worker memory spikes or OOM failures when
mutating rows.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=03b81444d32047198775f9f6a83558ec&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=03b81444d32047198775f9f6a83558ec&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/db_engine_specs/base.py
**Line:** 1239:1239
**Comment:**
*Performance: Unconditionally copying `data` to a new list when
mutators are present duplicates the full result set in memory even when
`fetchall()` already returned a mutable list. On large result sets this can
cause avoidable memory spikes and potential worker OOMs; only copy when the
returned container is immutable (for example, a tuple) so mutable list results
can be mutated in place.
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%2F42203&comment_hash=79133c037b8a45e4ef805a580b1b634bbb2f3b945d835fa16eeb7b6ff7460966&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42203&comment_hash=79133c037b8a45e4ef805a580b1b634bbb2f3b945d835fa16eeb7b6ff7460966&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]
