codeant-ai-for-open-source[bot] commented on code in PR #42054:
URL: https://github.com/apache/superset/pull/42054#discussion_r3590990561
##########
superset/utils/date_parser.py:
##########
@@ -98,6 +98,9 @@ def normalize_time_delta(human_readable: str) -> dict[str,
int]:
key = matched[2] + "s"
value = int(matched[1])
value = -value if matched[3] == "ago" else value
+ if key == "quarters":
+ # pd.DateOffset does not accept a `quarters` argument
+ key, value = "months", value * 3
Review Comment:
**Suggestion:** The quarter-conversion check is case-sensitive, but the
regex match is case-insensitive and preserves the user's original casing.
Inputs like `1 Quarter ago` or `1 QUARTER ago` will skip this branch, return
`{"Quarters": -1}`, and later fail when passed to `DateOffset`. Normalize the
unit key to lowercase before this comparison so quarter offsets work regardless
of input case. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Quarter-offset charts can crash during join-column generation.
- ⚠️ Grain-less offset alignment silently ignores quarter normalization
failures.
- ⚠️ Time comparison features unreliable for mixed-case quarter inputs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a timeseries chart using time comparison so the frontend sends
a quarter
offset string like "1 QUARTER ago" in `time_offsets` (see
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/buildQuery.ts:81-99`,
which
flows `formData.time_compare` into `time_offsets`).
2. The backend builds offset queries and then calls
`ExploreMixin.join_offset_dfs` from
`superset/models/helpers.py:2115-2122`, passing the main DataFrame,
`offset_dfs`, the
`time_grain`, and the join keys.
3. Inside `join_offset_dfs` (`superset/models/helpers.py:2434-2480`), for
each `offset`
string (e.g. "1 QUARTER ago") it delegates to `_determine_join_keys`, which
for relative
offsets without a custom producer eventually calls `add_offset_join_column`,
and that
applies `generate_join_column` per row
(`superset/models/helpers.py:2525-2580`).
4. In `generate_join_column` (`superset/models/helpers.py:2571-2581`), when
`time_offset`
is "1 QUARTER ago" and the value is datetime-like, the code executes `value
= value +
DateOffset(**normalize_time_delta(time_offset))` without a try/except;
`normalize_time_delta` at `superset/utils/date_parser.py:53-65` matches the
regex
case-insensitively, sets `key = matched[2] + "s"` yielding "QUARTERs", the
`if key ==
"quarters"` check fails because it is case-sensitive, and the function
returns
`{"QUARTERs": -1}`. Passing this dict to `DateOffset(**...)` raises a
`ValueError`/`TypeError` ("unexpected keyword argument 'QUARTERs'"), which
is not caught
here and bubbles up, causing the query to fail instead of producing a valid
quarter-shifted comparison series.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4f4db86cb09d4f1faf8494a111b6126a&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=4f4db86cb09d4f1faf8494a111b6126a&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/utils/date_parser.py
**Line:** 101:103
**Comment:**
*Incorrect Condition Logic: The quarter-conversion check is
case-sensitive, but the regex match is case-insensitive and preserves the
user's original casing. Inputs like `1 Quarter ago` or `1 QUARTER ago` will
skip this branch, return `{"Quarters": -1}`, and later fail when passed to
`DateOffset`. Normalize the unit key to lowercase before this comparison so
quarter offsets work regardless of input 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%2F42054&comment_hash=5f901d5e3997ef66b0bc9eb8dca8d7fbc237200454cd5789bb8c263fdbda48f0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=5f901d5e3997ef66b0bc9eb8dca8d7fbc237200454cd5789bb8c263fdbda48f0&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]
