codeant-ai-for-open-source[bot] commented on code in PR #41851:
URL: https://github.com/apache/superset/pull/41851#discussion_r3536397075
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -127,6 +127,13 @@ function getSortTypeByDataType(dataType: GenericDataType):
DefaultSortTypes {
return 'basic';
}
+/**
+ * Helper to determine if a column should be treated as a UUID
+ */
+function isUuidColumn(column?: { type?: string }): boolean {
+ return column?.type?.toLowerCase() === 'uuid';
+}
Review Comment:
**Suggestion:** The UUID check is too strict and only matches the exact
string `uuid`, so native UUID types expressed with wrappers or dialect-specific
names (for example nullable/wrapped UUID forms) will be misclassified as
searchable text and still produce invalid `ILIKE` SQL. Use a normalized
pattern-based check that recognizes UUID type variants. [incorrect condition
logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ UUID columns with wrapped types still cause query errors.
- ⚠️ Backends with nonplain UUID types remain partly broken.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use a dataset where the native UUID column is reported with a
dialect-specific type
string such as `Nullable(UUID)` in `props.datasource.columns`
(transformProps.ts builds
`columnsByName` from `props.datasource.columns` at lines 235-237).
2. When `processColumns` runs (transformProps.ts lines 239-339), it iterates
over
`colnames`, constructs `DataColumnMeta` for each column, and sets `type` from
`columnsByName.get(key)?.type` (line 327), so the UUID column’s `type` in
frontend
metadata becomes `Nullable(UUID)` rather than a plain `UUID`.
3. `TableChart.tsx` turns these metadata entries into React Table column
configs in
`getColumnConfigs` (lines 956-999 and 1393-1399), preserving the `type`
field, and the
`useEffect` at lines 1463-1475 then derives `searchOptions` by selecting
columns with
`sortType` equal to `alphanumeric` where `isUuidColumn(col)` is false.
4. The helper `isUuidColumn` only returns true when
`column.type.toLowerCase()` equals
exactly `uuid` (lines 130-135), so a type like `nullable(uuid)` is
misclassified as
non-UUID, the column is exposed in `searchOptions`, and server-side search
ultimately
drives an `ILIKE` filter on this UUID column via `buildQuery.ts` (lines
302-313),
recreating invalid SQL on backends such as ClickHouse that use wrapped UUID
type names.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d3ef73469a9949bf84dca5a937095ee0&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=d3ef73469a9949bf84dca5a937095ee0&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-frontend/plugins/plugin-chart-table/src/TableChart.tsx
**Line:** 133:135
**Comment:**
*Incorrect Condition Logic: The UUID check is too strict and only
matches the exact string `uuid`, so native UUID types expressed with wrappers
or dialect-specific names (for example nullable/wrapped UUID forms) will be
misclassified as searchable text and still produce invalid `ILIKE` SQL. Use a
normalized pattern-based check that recognizes UUID type variants.
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%2F41851&comment_hash=3ee5030f69e9beb3c3fbc2e61af4066385c8d8eb1a4440ab908d178864fa4512&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41851&comment_hash=3ee5030f69e9beb3c3fbc2e61af4066385c8d8eb1a4440ab908d178864fa4512&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-table/src/transformProps.ts:
##########
@@ -324,6 +324,7 @@ const processColumns = memoizeOne(function processColumns(
return {
key,
label,
+ type: columnsByName.get(key)?.type,
Review Comment:
**Suggestion:** Looking up column type only by `key` can fail when query
result column keys are aliased/renamed, which leaves `type` undefined and
prevents UUID exclusion logic from triggering. Add a fallback mapping path (for
example via original column metadata/verbose mapping) so aliased UUID columns
still carry their native type. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Aliased UUID dimensions remain searchable via server pagination.
- ⚠️ Adhoc UUID expressions bypass new UUID exclusion logic.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Table chart using an adhoc dimension that references a UUID
column via a
SQL expression, for example `sqlExpression: 'event_id'` with `label:
'event_uuid'`;
Superset treats this as an adhoc column, as indicated by the `isAdhocColumn`
import and
label mapping logic in transformProps.ts (lines 31-33 and 540-553).
2. When the query runs, the result `colnames` array includes the adhoc label
`event_uuid`;
`processColumns` iterates over `colnames` (transformProps.ts lines 239-246)
and uses each
`key` from `colnames` while building `DataColumnMeta`, but `columnsByName`
is keyed by
dataset `column_name` values from `props.datasource.columns` (lines
235-237), so there is
no entry matching the adhoc label.
3. Because no dataset column exists for the adhoc label,
`columnsByName.get(key)?.type` on
line 327 returns `undefined` for this UUID expression column, and the
resulting
`DataColumnMeta` passed into `TableChart.tsx` has `type` undefined even
though the
database column created by the expression is still a native UUID.
4. `TableChart.tsx` then builds React Table columns from this metadata in
`getColumnConfigs` (lines 956-999 and 1393-1399), and the `searchOptions`
effect (lines
1463-1475) filters columns based on `sortType` and `isUuidColumn`; since the
adhoc
column’s `dataType` is `GenericDataType.String` but its `type` is undefined,
it is treated
as a searchable text column, appears in `searchOptions`, and `buildQuery.ts`
at lines
302-313 subsequently adds an `ILIKE` filter against the UUID expression
column, leading to
invalid SQL for aliased UUID dimensions.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c2e280acd214448bb1ddabcdb1d35c2a&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=c2e280acd214448bb1ddabcdb1d35c2a&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-frontend/plugins/plugin-chart-table/src/transformProps.ts
**Line:** 327:327
**Comment:**
*Api Mismatch: Looking up column type only by `key` can fail when query
result column keys are aliased/renamed, which leaves `type` undefined and
prevents UUID exclusion logic from triggering. Add a fallback mapping path (for
example via original column metadata/verbose mapping) so aliased UUID columns
still carry their native type.
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%2F41851&comment_hash=6cc34dad5d904e6a1f86e3fe23504d5c753e7d223ab8bc627041bd7bd8039537&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41851&comment_hash=6cc34dad5d904e6a1f86e3fe23504d5c753e7d223ab8bc627041bd7bd8039537&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -1458,9 +1466,12 @@ export default function TableChart<D extends DataRecord
= DataRecord>(
{
columnKey: string;
sortType?: string;
+ type?: string;
}[]
)
- .filter(col => col?.sortType === 'alphanumeric')
+ .filter(
+ col => col?.sortType === 'alphanumeric' && !isUuidColumn(col),
+ )
Review Comment:
**Suggestion:** Filtering UUID columns out of `searchOptions` without also
reconciling an already-selected `searchColumn` leaves stale UUID values in
`ownState`. When a user had previously selected a UUID column, server-side
search still sends `ILIKE` against that stale column and continues to fail;
reset `searchColumn` to a valid option whenever the option list changes. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Server-side search still queries UUID column with ILIKE.
- ⚠️ Dropdown hides UUID but persisted selection still applied.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable server-side pagination on a legacy Table chart so
`serverPagination` is true and
the chart receives `ownState` as `serverPaginationData` with a UUID
`searchColumn` and
`searchText` (transformProps.ts lines 772-787, TableChart.tsx lines 386-400).
2. `processColumns` in transformProps.ts builds `DataColumnMeta` for each
query column and
attaches the native `type` from `columnsByName.get(key)?.type` (lines
239-239 and
325-328), which is then passed into `TableChart.tsx`.
3. In `TableChart.tsx`, `getColumnConfigs` converts these metadata entries
into React
Table columns and preserves the `type` field (lines 956-999 and 1393-1399),
and the
`useEffect` at lines 1463-1475 recomputes `searchOptions` by filtering to
columns with
`sortType` equal to `alphanumeric` and excluding `isUuidColumn(col)`, so the
UUID column
is no longer present in `searchOptions`.
4. Despite the filtered options, the `SearchSelectDropdown` in
`DataTable/DataTable.tsx`
still binds its value to `serverPaginationData?.searchColumn` (lines
586-592), and when
the user searches, `handleSearch` in TableChart.tsx (lines 1560-1568) builds
new ownState
with `searchColumn: serverPaginationData?.searchColumn ||
searchOptions[0]?.value`;
`buildQuery.ts` then adds an `ILIKE` filter using that stale UUID
`searchColumn` (lines
302-313), so server-side search continues to fail on the UUID column even
though it was
removed from the dropdown.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c265883161e2486aac5ab2b74b317769&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=c265883161e2486aac5ab2b74b317769&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-frontend/plugins/plugin-chart-table/src/TableChart.tsx
**Line:** 1472:1474
**Comment:**
*Logic Error: Filtering UUID columns out of `searchOptions` without
also reconciling an already-selected `searchColumn` leaves stale UUID values in
`ownState`. When a user had previously selected a UUID column, server-side
search still sends `ILIKE` against that stale column and continues to fail;
reset `searchColumn` to a valid option whenever the option list changes.
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%2F41851&comment_hash=09c46b2e9c21edbfda38315d193e482dccaddbcfaacbb52c01166267e75525f3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41851&comment_hash=09c46b2e9c21edbfda38315d193e482dccaddbcfaacbb52c01166267e75525f3&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]
