codeant-ai-for-open-source[bot] commented on code in PR #42597:
URL: https://github.com/apache/superset/pull/42597#discussion_r3679118253
##########
tests/unit_tests/queries/query_object_test.py:
##########
@@ -86,6 +86,34 @@ def
test_cache_key_changes_for_new_query_object_different_params():
assert query_object2.cache_key() != cache_key1
+def test_cache_key_stable_regardless_of_extra_cache_keys_order():
+ """
+ Regression for #34543: the cache key must not depend on the order of
+ ``extra_cache_keys``.
+
+ ``SqlaTable.get_extra_cache_keys`` (superset/connectors/sqla/models.py)
+ returns ``list(set(extra_cache_keys))``. Python's string hashing is
+ randomized per-process (``PYTHONHASHSEED``), so the same set of values
+ can iterate in a different order in the Celery worker process (which
+ writes the query results to cache) than in the web process (which
+ re-derives the cache key to read them back). Because ``hash_from_dict``
+ only sorts dict keys and not list values, two ``extra_cache_keys`` lists
+ with identical Jinja ``url_param()`` values but different order hash to
+ different cache keys, causing async chart-data lookups to 422 with
+ "Error loading data from cache" whenever more than one url_param is
+ referenced (a single-element list has only one possible order, which is
+ why the bug is only visible with multiple parameters).
+ """
+ query_object1 = QueryObject(row_limit=1)
+ query_object2 = QueryObject(row_limit=1)
+ same_values_different_order = ["CAR_IDS=1,2,3", "CHASSIS_IDS=100,200"]
+ cache_key1 =
query_object1.cache_key(extra_cache_keys=same_values_different_order)
+ cache_key2 = query_object2.cache_key(
+ extra_cache_keys=list(reversed(same_values_different_order))
+ )
Review Comment:
**Suggestion:** The fixture covers only strings, but real datasource output
contains heterogeneous hashable values such as strings, integers, and `None`. A
common fix for the ordering bug is sorting the deduplicated values, which
raises `TypeError` for mixed incomparable types; this test would pass while
leaving that production path broken. Include a mixed-type set of cache-key
components in the regression case. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed Jinja cache keys could cause query failures.
- ⚠️ Current regression fixture misses integer/string combinations.
- ⚠️ Deterministic sorting may raise runtime TypeError.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=37b0e8328cdf4bf198a1e63bc0470d45&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=37b0e8328cdf4bf198a1e63bc0470d45&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:** tests/unit_tests/queries/query_object_test.py
**Line:** 109:113
**Comment:**
*Type Error: The fixture covers only strings, but real datasource
output contains heterogeneous hashable values such as strings, integers, and
`None`. A common fix for the ordering bug is sorting the deduplicated values,
which raises `TypeError` for mixed incomparable types; this test would pass
while leaving that production path broken. Include a mixed-type set of
cache-key components in the regression case.
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%2F42597&comment_hash=451f9f867af639600c1f76e2bfc4decedec909f3ff03874293762e1cf4e90858&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42597&comment_hash=451f9f867af639600c1f76e2bfc4decedec909f3ff03874293762e1cf4e90858&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]
