codeant-ai-for-open-source[bot] commented on code in PR #41804:
URL: https://github.com/apache/superset/pull/41804#discussion_r3580498322
##########
superset/models/helpers.py:
##########
@@ -250,6 +250,25 @@ def json_to_dict(json_str: str) -> dict[Any, Any]:
return {}
+UUID_NATIVE_TYPE_RE: re.Pattern[str] = re.compile(r"\buuid\b", re.IGNORECASE)
Review Comment:
**Suggestion:** The UUID native-type detector is too narrow: it only matches
the literal word `uuid`, so SQL Server `uniqueidentifier` columns are not
recognized as UUID-like native types. Those columns map to string generic type,
so the new cast guard still skips casting and `LIKE`/`ILIKE` on them will
continue to fail at runtime. Expand the native-type detection to include other
UUID native names (at least `uniqueidentifier`) or switch to a more robust
UUID-type classification. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ MSSQL charts fail when filtering uniqueidentifier UUID columns.
- ⚠️ MSSQL UUID-like columns unusable in table search.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a SQL Server (MSSQL) database in Superset and expose a physical
dataset whose
underlying table has a column of native type `uniqueidentifier`. During
column
introspection, `MssqlEngineSpec.column_type_mappings` at
`superset/db_engine_specs/mssql.py:120-131` matches the regex
`^uniqueidentifier.*` and
maps it to the custom `GUID()` type with `GenericDataType.STRING`, which is
asserted in
`tests/unit_tests/db_engine_specs/test_mssql.py:44-19`. The `TableColumn`
ORM model stores
this native type string in its `type` field
(`superset/connectors/sqla/models.py:945-950`), so at runtime
`TableColumn.type ==
"uniqueidentifier"` for that column.
2. Build a table or similar chart on this MSSQL dataset, and from the UI
apply a filter or
server-side search on the `uniqueidentifier` column using a LIKE-family
operator (e.g.
contains/starts-with). This ultimately hits the `/api/v1/chart/<id>/data`
endpoint
implemented by `ChartDataRestApi.get_data` in
`superset/charts/data/api.py:86-165`, which
loads the saved `QueryContext` and, for SQLA datasets, constructs queries
that call
`SqlaTable.get_sqla_query` (`superset/connectors/sqla/models.py:2187-2193`),
delegating to
`BaseSqlaTable.get_sqla_query` in `superset/models/helpers.py:3294+`.
3. Inside `BaseSqlaTable.get_sqla_query` in
`superset/models/helpers.py:3748-3827`, while
iterating filters, `col_obj` is the `TableColumn` for the `uniqueidentifier`
column, so
`col_type = col_obj.type` yields the string `"uniqueidentifier"`
(`helpers.py:3755-3763`).
`db_engine_spec.get_column_spec(native_type=col_type)` uses the MSSQL column
mappings and
returns a spec whose `generic_type` is `GenericDataType.STRING` for this
native type.
Consequently `target_generic_type` is set to `GenericDataType.STRING` for
that column
(`helpers.py:3724-3734`).
4. When the operator is one of `{ILIKE, LIKE, NOT_LIKE, NOT_ILIKE}`, the new
LIKE-handling
branch at `superset/models/helpers.py:3881-3895` computes
`needs_string_cast_for_like =
(target_generic_type != GenericDataType.STRING or
is_uuid_native_type(col_type))`. For the
MSSQL `uniqueidentifier` column, `target_generic_type ==
GenericDataType.STRING` and
`is_uuid_native_type("uniqueidentifier")` returns `False` because
`UUID_NATIVE_TYPE_RE` is
`re.compile(r"\buuid\b", re.IGNORECASE)` (`helpers.py:253-269)`, which only
matches the
whole word `uuid` and does not match the substring inside
`uniqueidentifier`. Therefore
`needs_string_cast_for_like` is `False`, no `sa.cast(sqla_col, sa.String)`
is applied, and
Superset generates a predicate like `WHERE guid_col LIKE '%abc%'` against a
`uniqueidentifier` column. On SQL Server such `LIKE` operations on
`uniqueidentifier`
without an explicit cast are rejected with a type error, so the
`/api/v1/chart/<id>/data`
request for this chart fails at runtime whenever a LIKE-family filter is
applied to a
`uniqueidentifier` column. Expanding `is_uuid_native_type` to recognize
`uniqueidentifier`
(or a broader UUID-native set) would cause `needs_string_cast_for_like` to
be `True`, add
the necessary cast, and avoid this failure.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bdd7cbc5c2944272b0e6fcd5a5787802&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=bdd7cbc5c2944272b0e6fcd5a5787802&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/models/helpers.py
**Line:** 253:253
**Comment:**
*Incomplete Implementation: The UUID native-type detector is too
narrow: it only matches the literal word `uuid`, so SQL Server
`uniqueidentifier` columns are not recognized as UUID-like native types. Those
columns map to string generic type, so the new cast guard still skips casting
and `LIKE`/`ILIKE` on them will continue to fail at runtime. Expand the
native-type detection to include other UUID native names (at least
`uniqueidentifier`) or switch to a more robust UUID-type classification.
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%2F41804&comment_hash=26913d00a6030bb0c85594a300a23fc0e77859ed04c8b8c9fd927cfce32b9220&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41804&comment_hash=26913d00a6030bb0c85594a300a23fc0e77859ed04c8b8c9fd927cfce32b9220&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]
