codeant-ai-for-open-source[bot] commented on code in PR #41580:
URL: https://github.com/apache/superset/pull/41580#discussion_r3503473149
##########
superset/daos/datasource.py:
##########
@@ -130,6 +150,21 @@ def build_dataset_query(
if database_id is not None:
ds_q = ds_q.where(SqlaTable.database_id == database_id)
+ if schema_filter is not None:
+ ds_q = ds_q.where(SqlaTable.schema == schema_filter)
+
+ if owners_filter is not None:
+ ds_q = ds_q.join(
+ sqla_models.sqlatable_user,
+ sqla_models.sqlatable_user.c.table_id == ds_table.c.id,
+ ).where(sqla_models.sqlatable_user.c.user_id.in_(owners_filter))
Review Comment:
**Suggestion:** Filtering by multiple owners introduces a many-to-many join
without deduplicating dataset rows, so a dataset that has two matching owners
will be returned multiple times and the total count will be inflated. Apply the
owner filter via an EXISTS/IN subquery (or enforce DISTINCT on dataset id) so
each dataset appears once regardless of how many matching owner rows exist.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ Combined datasource list returns duplicate rows for multi-owner filters.
⚠️ Total count field inflated, breaking pagination and page indicators.
⚠️ Behaviour inconsistent with dataset endpoint owner filtering subquery
implementation.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the combined list API endpoint implemented by
`DatasourceRestApi.combined_list` in
`superset/datasource/api.py:521-569`, issuing a GET request with a
Rison-encoded `q` query
parameter matching `get_list_schema` that includes a `filters` entry for
owners: `{"col":
"owners", "opr": "rel_m_m", "value": [OWNER_A_ID, OWNER_B_ID]}` where a
dataset is owned
by both OWNER_A_ID and OWNER_B_ID (owners are stored via the
`sqlatable_user` mapping
table used elsewhere in `superset/connectors/sqla/models.py` and referenced
in
`superset/daos/dataset.py:7-14`).
2. The request reaches `GetCombinedDatasourceListCommand.run` in
`superset/commands/datasource/list.py:67-113`, which parses filters via
`_parse_filters`
at `list.py:251-279`; `_apply_filter` at `list.py:214-249` handles the `col
== "owners"`
and `opr == "rel_m_m"` case, calling `_apply_owners_filter` at
`list.py:53-64` to convert
the list `[OWNER_A_ID, OWNER_B_ID]` into `owners_filter=[OWNER_A_ID,
OWNER_B_ID]` on the
`_Filters` dataclass.
3. `run()` then calls `_build_combined_query` at `list.py:133-152`, which
invokes
`DatasourceDAO.build_dataset_query` with
`owners_filter=filters.owners_filter` (see
`list.py:134-143` and `superset/daos/datasource.py:109-168`). Inside
`build_dataset_query`, when `owners_filter` is not None, the code at
`datasource.py:156-160` performs an inner join on
`sqla_models.sqlatable_user` and applies
`.where(sqla_models.sqlatable_user.c.user_id.in_(owners_filter))`. Because
the dataset has
two matching rows in `sqlatable_user` (one per owner), this join produces
two identical
dataset rows (same `item_id`) in `ds_q` for that single dataset.
4. `_build_combined_query` unions `ds_q` with `sv_q` via `union_all(ds_q,
sv_q).subquery()` at `list.py:148-152`, and
`DatasourceDAO.paginate_combined_query` at
`superset/daos/datasource.py:205-239` is called with this combined subquery.
`paginate_combined_query` computes `total_count` using
`select(func.count()).select_from(combined)` at `datasource.py:225-227` and
returns
paginated rows via `select(combined.c.item_id, combined.c.source_type)` at
`datasource.py:232-237`. Because the joined dataset appears twice in
`combined` for
OWNER_A_ID and OWNER_B_ID, `total_count` is inflated (e.g., 2 instead of 1)
and the rows
list contains duplicate entries with the same `item_id`, leading
`_serialize_rows` in
`list.py:154-174` to serialize the same dataset object multiple times in the
`result`
array.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=81c91c6a0b6643f1ae17f6ff50e7da5f&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=81c91c6a0b6643f1ae17f6ff50e7da5f&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/daos/datasource.py
**Line:** 156:160
**Comment:**
*Logic Error: Filtering by multiple owners introduces a many-to-many
join without deduplicating dataset rows, so a dataset that has two matching
owners will be returned multiple times and the total count will be inflated.
Apply the owner filter via an EXISTS/IN subquery (or enforce DISTINCT on
dataset id) so each dataset appears once regardless of how many matching owner
rows exist.
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%2F41580&comment_hash=1701018e84b62f87bf3e8bf8f33e9fe54e72e0f1b99d132f7af3dbd9d4dad67e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41580&comment_hash=1701018e84b62f87bf3e8bf8f33e9fe54e72e0f1b99d132f7af3dbd9d4dad67e&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]
