codeant-ai-for-open-source[bot] commented on code in PR #37865:
URL: https://github.com/apache/superset/pull/37865#discussion_r2790662063
##########
superset/daos/dataset.py:
##########
@@ -49,6 +52,37 @@ class DatasetDAO(BaseDAO[SqlaTable]):
base_filter = DatasourceFilter
+ @classmethod
+ def apply_column_operators(
+ cls,
+ query: Query,
+ column_operators: list[ColumnOperator] | None = None,
+ ) -> Query:
+ """Override to handle database_name filter via join to Database table.
+
+ database_name lives on Database, not SqlaTable, so we intercept it
+ here, join to the Database table, and apply the filter there.
+ """
+ if not column_operators:
+ return query
+
+ remaining_operators: list[ColumnOperator] = []
+ for c in column_operators:
+ if not isinstance(c, ColumnOperator):
+ continue
Review Comment:
**Suggestion:** The custom `apply_column_operators` implementation silently
skips any filter that is not already a `ColumnOperator` instance, whereas the
base DAO logic supports plain dicts and other compatible inputs; this means
dataset filters passed as dictionaries (including `database_name` filters in
non-MCP code paths) will be ignored and not applied, breaking expected
filtering behavior. Normalize non-`ColumnOperator` inputs instead of dropping
them so all callers continue to work. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dataset filters passed as dicts are silently ignored.
- ⚠️ MCP list_datasets may not honor provided filters.
- ⚠️ Future callers relying on dict operators will misbehave.
```
</details>
```suggestion
# Normalize to ColumnOperator so dicts and other compatible
inputs are handled
if not isinstance(c, ColumnOperator):
c = ColumnOperator.model_validate(c)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset/daos/base.py`, `BaseDAO.apply_column_operators()` (called
from its
filtering helpers such as `apply_all_filters`/`get_list`) is implemented to
accept both
`ColumnOperator` instances and plain dicts, normalizing dicts via
`ColumnOperator.model_validate(...)`. This establishes a contract that DAO
subclasses must
also handle dict-based operators.
2. For datasets, the subclass `DatasetDAO` in
`superset/daos/dataset.py:56-84` overrides
`apply_column_operators()` and becomes the implementation used whenever
filters are
applied to dataset queries (including from REST APIs and MCP tools that call
into
`DatasetDAO` via the base DAO helpers).
3. Any caller that passes filter payloads as dicts—for example a list like
`[{"col":
"table_name", "opr": "eq", "value": "my_table"}]` coming from request JSON
or from the MCP
dataset filter schema—will reach `DatasetDAO.apply_column_operators()` with
`column_operators` containing dicts instead of `ColumnOperator` instances.
4. Inside `DatasetDAO.apply_column_operators()` the loop `for c in
column_operators:`
checks `if not isinstance(c, ColumnOperator): continue`, so every dict is
skipped, never
added to `remaining_operators`, and neither the custom `database_name`
branch nor
`super().apply_column_operators(...)` are invoked; as a result, the
underlying SQLAlchemy
`Query` is returned with **no filters applied at all**, so dataset filter
requests that
rely on dict-style operators are silently ignored.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/daos/dataset.py
**Line:** 72:72
**Comment:**
*Logic Error: The custom `apply_column_operators` implementation
silently skips any filter that is not already a `ColumnOperator` instance,
whereas the base DAO logic supports plain dicts and other compatible inputs;
this means dataset filters passed as dictionaries (including `database_name`
filters in non-MCP code paths) will be ignored and not applied, breaking
expected filtering behavior. Normalize non-`ColumnOperator` inputs instead of
dropping them so all callers continue to work.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37865&comment_hash=cf8e5b545fe45da06d7a559acda935e9d3df7835cd6db00dea61271738d4f20d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37865&comment_hash=cf8e5b545fe45da06d7a559acda935e9d3df7835cd6db00dea61271738d4f20d&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]
