richardfogaca commented on PR #40682:
URL: https://github.com/apache/superset/pull/40682#issuecomment-4604204624
_Posting on Richard's behalf — this is his PR reviewer agent. Forward any
pushback to him and he'll loop me back in._
Left one functional note below. The direct `search="%"` path is fixed, but
the MCP list filter path still appears to expose the same wildcard behavior
through `column_operators`. Line numbers verified against HEAD `49eebad`.
### Functional — worth fixing before merge
- **`superset/daos/base.py:667`**
This escapes the free-text `search` branch, but MCP filters still pass
through `ModelListCore` into `BaseDAO.list(column_operators=...)`, and the
shared `operator_map` still builds raw wildcard patterns for `sw`, `ew`, `ct`,
`like`, and `ilike` at `superset/daos/base.py:87-101`. For example, a caller
can still send a list filter like `{"col": "dashboard_title", "opr": "ilike",
"value": "%"}`; the dashboard schema accepts `ColumnOperatorEnum` for `opr`,
and schema discovery advertises filterable operators, so that still becomes
`ILIKE '%%'`.
WDYT — could we route the wildcard operators through the same escaping
logic, or otherwise restrict wildcard operators for MCP list filters? It would
also be worth adding a regression for `column_operators` with `%` / `_`, since
the new test only covers the `search` parameter.
### Praise
- **`tests/integration_tests/dao/base_dao_test.py:812`**
The direct `search="%"` regression is a good target for the reported issue
and keeps the test at the DAO layer where both `_build_query` / `list` behavior
is easiest to reason about.
--
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]
