You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "yousoph (via GitHub)" <gi...@apache.org> on 2026/06/16 05:24:26 UTC
[PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
yousoph opened a new pull request, #41099:
URL: https://github.com/apache/superset/pull/41099
### Summary
When a database driver returns Python `dict` or `list` objects as cell values (e.g. ClickHouse's native JSON and Map types via clickhouse-connect), Superset's `stringify_values()` in `result_set.py` converted them via `obj.astype(str)`, which calls Python's `str()` built-in and produces single-quoted repr:
```
{'a': {'b': 42}, 'c': [1, 2, 3]}
```
This is not valid JSON. The SQL Lab cell viewer calls `safeJsonObjectParse()` (which uses `JSONbig.parse()` internally) on cell values — it fails silently on single-quoted strings, so the interactive tree viewer never activates for those cells.
The fix: in `stringify_values()`, when the extracted Python value is a `dict` or `list`, use the existing `stringify()` helper (which calls `json.dumps`) to produce valid double-quoted JSON:
```
{"a": {"b": 42}, "c": [1, 2, 3]}
```
This makes the cell viewer work for ClickHouse native JSON/Map columns and any other driver that returns dict/list objects.
### Reproduction
Using ClickHouse 25.3+ with either `clickhouse-connect` or `clickhouse-sqlalchemy`:
```sql
WITH '{"a":{"b":42},"c":[1,2,3]}'::JSON AS json
SELECT json, json::String
```
Before: clicking the `json` cell opens nothing. After: opens the interactive tree viewer.
### Testing
- `tests/unit_tests/result_set_test.py::test_stringify_values_dict_and_list_produce_valid_json` — unit test for the `stringify_values` change
- `tests/unit_tests/result_set_test.py::test_clickhouse_json_column_in_pa_table_is_valid_json` — integration-style test using `SupersetResultSet` with ClickHouse-style dict rows
### Checklist
- [x] Unit tests added/updated
- [x] All existing `result_set_test.py` tests pass
- [x] No frontend changes required — fix is entirely in the serialization path
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41099:
URL: https://github.com/apache/superset/pull/41099#issuecomment-4723787282
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 948382cf8bf5a3c18b4d083e68a9598c5677da96 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a31bf208b30d500081f478d |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-41099--superset-docs-preview.netlify.app](https://deploy-preview-41099--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcrJDYAgEADAXvYtyXoTO7AMjiWiKAQ8Y-xdfvOYF0L0a9hhAChARrGpKdvYmyln1TL5IxGbk9-YIufYaemimKum4Pwz6pw7UZfSVMhljbpFRF6apucavh8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41099--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "yousoph (via GitHub)" <gi...@apache.org>.
yousoph merged PR #41099:
URL: https://github.com/apache/superset/pull/41099
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41099:
URL: https://github.com/apache/superset/pull/41099#discussion_r3418341711
##########
superset/result_set.py:
##########
@@ -73,9 +73,16 @@ def stringify_values(array: NDArray[Any]) -> NDArray[Any]:
obj[na_obj] = None
else:
try:
- # for simple string conversions
- # this handles odd character types better
- obj[...] = obj.astype(str)
+ val = obj.item()
+ if isinstance(val, (dict, list)):
+ # Use json.dumps to produce valid double-quoted JSON.
+ # Python's str() gives single-quoted repr like {'a': 1}
+ # which is not valid JSON and breaks the frontend cell viewer.
+ obj[...] = stringify(val)
Review Comment:
**Suggestion:** `stringify(val)` can raise `TypeError` for dict/list values containing non-JSON-serializable members (for example bytes or custom objects), but this branch only catches `ValueError`, so result-set construction can now fail at runtime instead of falling back to string conversion. Catch `TypeError` in this path (or use a pessimistic serializer/fallback to `str`) to preserve previous non-crashing behavior. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ SQL Lab queries crash on nested unserializable objects.
- ⚠️ Dashboards using those queries fail to render results.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Note that `stringify()` in `superset/result_set.py:62-63` calls
`superset.utils.json.dumps(obj, default=json.json_iso_dttm_ser)`, and `json_iso_dttm_ser`
in `superset/utils/json.py:114-137` delegates to `base_json_conv`
(`superset/utils/json.py:73-112`), which raises `TypeError` for any object it cannot
convert (final `raise TypeError(f"Unserializable object {obj}...")` at line 111).
2. In `SupersetResultSet.__init__` (`superset/result_set.py:167-200, 212-239`), when Arrow
conversion fails for a column, the code falls back to stringification: inside the `except
(pa.lib.ArrowInvalid, pa.lib.ArrowTypeError, pa.lib.ArrowNotImplementedError, ValueError,
TypeError)` block, it calls `stringify_values(columns[column])` at
`superset/result_set.py:237`.
3. In `stringify_values` (`superset/result_set.py:66-89`), for each cell it does `val =
obj.item()` (line 76) and, if `val` is a `dict` or `list`, executes `obj[...] =
stringify(val)` (line 81). This call is wrapped in `try:` with an `except ValueError:`
handler only (line 86), so any `TypeError` raised by `stringify(val)` will **not** be
caught.
4. To reproduce, add a unit test similar to
`tests/unit_tests/result_set_test.py::test_stringify_with_null_integers` that constructs
`data = [({"k": CustomType()},)]` where `CustomType` is a user-defined class not handled
by `base_json_conv`, and a matching `cursor_description`. When the test instantiates
`SupersetResultSet(data, description, BaseEngineSpec)` (see existing usage in
`tests/unit_tests/result_set_test.py:73` and production callers in
`superset/sql/execution/executor.py:162` and `superset/sql_lab.py:340`), the Arrow
conversion for that column will fail, the fallback at `superset/result_set.py:237` will
call `stringify_values`, and `stringify(val)` will attempt to serialize the nested
`CustomType` instance. `json_iso_dttm_ser`/`base_json_conv` will raise `TypeError`, which
propagates out of `stringify_values` (not caught by `except ValueError`) and out of
`SupersetResultSet.__init__`, causing the SQL Lab query (or any caller through
`SQLExecutor` / Celery `_execute_sql_statements`) to fail at runtime instead of degrading
to a plain string representation as before.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7a4ca9ccd0db463385efd6747b0dfb22&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=7a4ca9ccd0db463385efd6747b0dfb22&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/result_set.py
**Line:** 76:81
**Comment:**
*Type Error: `stringify(val)` can raise `TypeError` for dict/list values containing non-JSON-serializable members (for example bytes or custom objects), but this branch only catches `ValueError`, so result-set construction can now fail at runtime instead of falling back to string conversion. Catch `TypeError` in this path (or use a pessimistic serializer/fallback to `str`) to preserve previous non-crashing behavior.
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%2F41099&comment_hash=a8cbf5fbf9bc326478a7c5cd1fd6f3a0ad73232a7b599cd9f5a80b1db21c2b26&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41099&comment_hash=a8cbf5fbf9bc326478a7c5cd1fd6f3a0ad73232a7b599cd9f5a80b1db21c2b26&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41099:
URL: https://github.com/apache/superset/pull/41099#issuecomment-4724806350
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41099:
URL: https://github.com/apache/superset/pull/41099#issuecomment-4715264583
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41099:
URL: https://github.com/apache/superset/pull/41099#issuecomment-4715298217
<!-- Bito Reply -->
The flagged issue is correct. The current implementation of `stringify_values` in `superset/result_set.py` only catches `ValueError` when calling `stringify(val)`, which causes the process to crash if `stringify` raises a `TypeError` (e.g., for non-JSON-serializable objects).
To resolve this, you should update the `try...except` block to catch `TypeError` as well. Here is the corrected implementation for `superset/result_set.py`:
```python
try:
val = obj.item()
if isinstance(val, (dict, list)):
obj[...] = stringify(val)
else:
obj[...] = obj.astype(str)
except (ValueError, TypeError):
obj[...] = stringify(obj)
```
This change ensures that if `stringify(val)` fails due to a `TypeError`, the code falls back to the original `stringify(obj)` behavior, preventing runtime crashes.
**superset/result_set.py**
```
try:
val = obj.item()
if isinstance(val, (dict, list)):
obj[...] = stringify(val)
else:
obj[...] = obj.astype(str)
except (ValueError, TypeError):
obj[...] = stringify(obj)
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41099:
URL: https://github.com/apache/superset/pull/41099#issuecomment-4735466779
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41099:
URL: https://github.com/apache/superset/pull/41099#issuecomment-4715324297
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41099?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `2 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 63.53%. Comparing base ([`7b4efac`](https://app.codecov.io/gh/apache/superset/commit/7b4efacbc2e68e2b740d880b8615cd631a72c06a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`01464a4`](https://app.codecov.io/gh/apache/superset/commit/01464a49b388d3d57c755ed73e9c3de18581ee12?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/41099?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/result\_set.py](https://app.codecov.io/gh/apache/superset/pull/41099?src=pr&el=tree&filepath=superset%2Fresult_set.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | 50.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/superset/pull/41099?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41099 +/- ##
==========================================
- Coverage 64.32% 63.53% -0.79%
==========================================
Files 2652 2652
Lines 144728 144731 +3
Branches 33384 33385 +1
==========================================
- Hits 93090 91958 -1132
- Misses 49985 51114 +1129
- Partials 1653 1659 +6
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.33% <50.00%> (-0.01%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.91% <50.00%> (-0.01%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.89% <50.00%> (-1.67%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.73% <50.00%> (-0.01%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/41099/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41099?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sql_lab): serialize dict/list cell values as valid JSON strings [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #41099:
URL: https://github.com/apache/superset/pull/41099#discussion_r3418733327
##########
superset/result_set.py:
##########
@@ -73,9 +73,16 @@ def stringify_values(array: NDArray[Any]) -> NDArray[Any]:
obj[na_obj] = None
else:
try:
- # for simple string conversions
- # this handles odd character types better
- obj[...] = obj.astype(str)
+ val = obj.item()
+ if isinstance(val, (dict, list)):
+ # Use json.dumps to produce valid double-quoted JSON.
+ # Python's str() gives single-quoted repr like {'a': 1}
+ # which is not valid JSON and breaks the frontend cell viewer.
+ obj[...] = stringify(val)
+ else:
+ # for simple string conversions
+ # this handles odd character types better
+ obj[...] = obj.astype(str)
Review Comment:
`stringify_values` now JSON-serializes `dict`/`list` via `stringify(val)`, which can raise `TypeError` for nested values that aren't handled by `json_iso_dttm_ser`. This introduces a new failure mode during result set materialization where previously `astype(str)` would always succeed. Consider catching `TypeError` and falling back to a pessimistic JSON serializer (or `str()`) to avoid breaking query results.
##########
tests/unit_tests/result_set_test.py:
##########
@@ -450,3 +450,71 @@ def test_stringify_extension_columns() -> None:
# plain binary BLOBs and other types are left untouched
assert pa.types.is_binary(result.schema.field("blob").type)
assert pa.types.is_integer(result.schema.field("n").type)
+
+
+def test_stringify_values_dict_and_list_produce_valid_json() -> None:
+ """
+ ClickHouse native JSON and Map types return Python dicts. When stringified for
+ Arrow array storage they must produce valid double-quoted JSON strings, not
+ Python's single-quoted repr. Single-quoted strings pass the cheap '{' prefix
+ check in the frontend's safeJsonObjectParse but then fail JSONbig.parse(),
+ so the SQL Lab cell viewer never activates.
+ """
+ data = np.array(
+ [
+ {"key": "value", "nested": {"a": 1}},
+ [1, 2, 3],
+ {"items": [1, 2, 3], "d": "Hello, World!"},
+ None,
+ ],
+ dtype=object,
+ )
+ result = stringify_values(data)
+
+ # Must be valid JSON strings (double-quoted), not Python repr (single-quoted)
+ assert result[0] == '{"key": "value", "nested": {"a": 1}}'
+ assert result[1] == "[1, 2, 3]"
+ assert result[2] == '{"items": [1, 2, 3], "d": "Hello, World!"}'
+ assert result[3] is None
+
+ # Parseable by a JSON parser — confirms the frontend's JSON.parse would succeed
+ parsed = superset_json.loads(result[0])
+ assert parsed == {"key": "value", "nested": {"a": 1}}
+ parsed = superset_json.loads(result[1])
+ assert parsed == [1, 2, 3]
Review Comment:
The list case in this test uses only integers (`[1, 2, 3]`), where Python's repr is already valid JSON. This means the test can still pass even if lists containing strings are incorrectly serialized with single quotes (the original bug). Consider using a list with strings and asserting via `loads()`/prefix checks instead of exact JSON formatting.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org