codeant-ai-for-open-source[bot] commented on code in PR #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3293884062
##########
superset/daos/base.py:
##########
@@ -499,9 +499,24 @@ def apply_column_operators(
)
column = getattr(cls.model_cls, col)
try:
- # Always use ColumnOperatorEnum's apply method
operator_enum = ColumnOperatorEnum(opr)
- query = query.filter(operator_enum.apply(column, value))
+ # Relationship attributes (many-to-one or many-to-many)
+ # can't be compared directly with scalar operators —
+ # SQLAlchemy needs `.any(...)` or `.has(...)`. Detect the
+ # collection case and dispatch to the related model's id
+ # column. This lets callers use the natural shape
+ # `{col: "<relationship>", opr: "eq", value: <id>}` to
+ # find rows whose related collection contains the given id.
+ if (
+ operator_enum == ColumnOperatorEnum.eq
+ and hasattr(column, "property")
+ and isinstance(column.property, RelationshipProperty)
+ and column.property.uselist
+ ):
+ related_pk = inspect(column.property.mapper).primary_key[0]
+ query = query.filter(column.any(related_pk == value))
+ else:
+ query = query.filter(operator_enum.apply(column, value))
Review Comment:
**Suggestion:** Relationship filters only get special handling for `eq`; any
other operator on a collection relationship still falls through to the scalar
operator path and will raise SQLAlchemy runtime errors (for example when a
client sends `opr="in"` or `opr="sw"` with `col="dashboards"`). Add explicit
handling for allowed relationship operators (or raise a clear validation error
before query execution) so relationship filters cannot hit the invalid scalar
comparison path. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ list_charts crashes when using non-eq dashboard filters.
- ⚠️ MCP clients can trigger 500s with malformed relationship filters.
- ⚠️ Any BaseDAO m2m filter misusing operators hits runtime errors.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. From an MCP client, call the `list_charts` tool defined in
`superset/mcp_service/chart/tool/list_charts.py:14-46` with a request
payload whose
`filters` array contains an entry like `{"col": "dashboards", "opr": "in",
"value": [42]}`
(or any non-`eq` operator such as `"sw"`).
2. Inside `list_charts`, the request is parsed into `ListChartsRequest` and
`ChartFilter`
objects (see `superset/mcp_service/chart/schemas.py:18-48`, where
`ChartFilter.opr` is a
`ColumnOperatorEnum` and `col` allows `"dashboards"`). The tool then
constructs a
`ModelListCore` with `dao_class=ChartDAO` and calls
`tool.run_tool(filters=request.filters, ...)` (see
`superset/mcp_service/chart/tool/list_charts.py:93-114`).
3. `ModelListCore.run_tool` in `superset/mcp_service/mcp_core.py:260-38`
forwards those
`ChartFilter` objects to the DAO via
`ChartDAO.list(column_operators=filters, ...)`.
`ChartDAO` inherits `BaseDAO[Slice]` (`superset/daos/chart.py`) so
`BaseDAO.apply_column_operators` is used to apply the filters.
4. In `BaseDAO.apply_column_operators` (`superset/daos/base.py:18-64` and
the new logic at
lines 510-519), for the filter with `col="dashboards"` and `opr="in"`,
`column` is the
`Slice.dashboards` many-to-many relationship attribute, `operator_enum` is
`ColumnOperatorEnum.in_`, the relationship-specific `eq` branch at lines
510-56 is
skipped, and execution falls into the `else` branch at line 519 calling
`operator_enum.apply(column, value)`. `ColumnOperatorEnum.in_` is mapped in
`superset/daos/base.py:29-37` to `lambda col, val: col.in_(...)`, which is
invalid for a
collection relationship attribute; when `query.filter(column.in_(...))` is
evaluated,
SQLAlchemy raises a runtime error (e.g. an AttributeError or StatementError
about
comparing a collection to a scalar), causing the `list_charts` MCP call to
fail with an
internal error instead of returning results or a clear validation error.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e6f6b28fd1ca4f6bb024b8cfb8219af3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e6f6b28fd1ca4f6bb024b8cfb8219af3&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/base.py
**Line:** 510:519
**Comment:**
*Logic Error: Relationship filters only get special handling for `eq`;
any other operator on a collection relationship still falls through to the
scalar operator path and will raise SQLAlchemy runtime errors (for example when
a client sends `opr="in"` or `opr="sw"` with `col="dashboards"`). Add explicit
handling for allowed relationship operators (or raise a clear validation error
before query execution) so relationship filters cannot hit the invalid scalar
comparison path.
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%2F40397&comment_hash=33247a3251c46da1a0f0c3ae4dc157e0647835c4d555223bd9fddbd615fe5795&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=33247a3251c46da1a0f0c3ae4dc157e0647835c4d555223bd9fddbd615fe5795&reaction=dislike'>👎</a>
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -538,12 +538,15 @@ class ChartFilter(ColumnOperator):
"datasource_name",
"created_by_fk",
"changed_by_fk",
+ "dashboards",
Review Comment:
**Suggestion:** The schema now accepts `dashboards`, but this addition is
not reflected in DAO filter discovery metadata, so
`get_schema(model_type='chart')` still does not advertise `dashboards` as a
filter column. This creates an API contract mismatch where clients following
schema discovery won't learn about the new filter. Update chart filter metadata
exposure (or relationship filter discovery) so `dashboards` appears
consistently in `get_schema`. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ get_schema omits dashboards, hiding a supported chart filter.
- ⚠️ MCP agents relying on discovery cannot learn dashboards filter.
- ⚠️ Documentation in ChartFilter contradicts actual schema metadata.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the MCP `get_schema` tool with `{"request": {"model_type":
"chart"}}`, which is
implemented in `superset/mcp_service/system/tool/get_schema.py:66-83`. For
`model_type="chart"`, `_get_chart_schema_core` returns a
`ModelGetSchemaCore` configured
with `dao_class=ChartDAO`, `select_columns=get_chart_columns()`, and
`exclude_filter_columns=set(SELF_REFERENCING_FILTER_COLUMNS)`.
2. In `ModelGetSchemaCore.run_tool`
(`superset/mcp_service/mcp_core.py:69-83`), the method
`_get_filter_columns` at `superset/mcp_service/mcp_core.py:35-62` calls
`ChartDAO.get_filterable_columns_and_operators()` to build
`schema_info.filter_columns`.
3. `ChartDAO.get_filterable_columns_and_operators`
(`superset/daos/chart.py:92-16`) simply
delegates to `BaseDAO.get_filterable_columns_and_operators` and then
augments with
`CHART_CUSTOM_FIELDS`. `BaseDAO.get_filterable_columns_and_operators` in
`superset/daos/base.py:526-119` inspects `mapper.columns` (real table
columns) and hybrid
properties; it does not include relationship attributes like the many-to-many
`Slice.dashboards` relationship, so `"dashboards"` never appears in the
returned mapping.
4. As a result, the `get_schema` response for charts contains
`schema_info.filter_columns`
with entries for fields like `"slice_name"` and `"viz_type"`, but no
`"dashboards"` key,
while the MCP request schema `ChartFilter` in
`superset/mcp_service/chart/schemas.py:18-40` explicitly allows
`col="dashboards"` and its
description tells clients to "Use get_schema(model_type='chart') for
available filter
columns" and mentions filtering by `'dashboards'`. This creates a contract
mismatch:
clients following the documented discovery flow (call `get_schema` to learn
valid filters,
then call `list_charts`) cannot discover the new `dashboards` filter even
though the list
tool and DAO now support it.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a058c1b7a028453bb51605c967517650&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a058c1b7a028453bb51605c967517650&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/mcp_service/chart/schemas.py
**Line:** 541:541
**Comment:**
*Api Mismatch: The schema now accepts `dashboards`, but this addition
is not reflected in DAO filter discovery metadata, so
`get_schema(model_type='chart')` still does not advertise `dashboards` as a
filter column. This creates an API contract mismatch where clients following
schema discovery won't learn about the new filter. Update chart filter metadata
exposure (or relationship filter discovery) so `dashboards` appears
consistently in `get_schema`.
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%2F40397&comment_hash=b521233f6de47e872fa18f2b9fb3f3958477c2f533a7ed9c6aeef7d3a0d6b31c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=b521233f6de47e872fa18f2b9fb3f3958477c2f533a7ed9c6aeef7d3a0d6b31c&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]
