codeant-ai-for-open-source[bot] commented on code in PR #38716:
URL: https://github.com/apache/superset/pull/38716#discussion_r3658138778
##########
superset/models/helpers.py:
##########
@@ -3981,6 +4025,15 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
template_processor=template_processor
)
is_metric_filter = True
+ elif col_obj is None and isinstance(flt_col, str):
+ sqla_col = self.find_adhoc_column_and_convert_to_sqla(
+ columns=columns,
+ label=flt_col,
+ template_processor=template_processor,
+ )
+ if sqla_col is not None:
+ applied_adhoc_filters_columns.append(flt_col)
+
applied_template_filters.append(get_column_name(flt_col))
Review Comment:
**Suggestion:** The renamed adhoc column is appended to
`applied_adhoc_filters_columns` as a plain label string, whereas the existing
adhoc path appends the original adhoc column object. Any later logic that
compares or processes this collection as adhoc column definitions will not
recognize the string, which can cause the filter to be retained or processed
incorrectly. Store the original adhoc filter column definition, or update all
consumers to consistently handle labels. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Renamed adhoc filters can be mishandled during table chart query
bookkeeping.
- ⚠️ Server-side pagination searches may retain duplicate or incorrect
filters.
- ⚠️ Adhoc-column filter processing receives inconsistent object types.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a Superset table chart with server-side pagination and a
searchable dimension
whose adhoc SQL expression has a renamed label, following the documented
chart setup.
2. Request chart data through the table chart data flow, which reaches
`get_sqla_query()`
in `superset/models/helpers.py`; for the renamed search field, `flt_col` is
the displayed
label rather than the original adhoc-column dictionary.
3. At `superset/models/helpers.py:4006-4036`, the regular-column and metric
lookups fail,
then `find_adhoc_column_and_convert_to_sqla()` resolves the label to a
SQLAlchemy
expression at lines 4029-4034.
4. The new branch records the plain label string in
`applied_adhoc_filters_columns` at
line 4035, while the existing adhoc branch records the original adhoc-column
definition at
line 4000; later filter bookkeeping in the same `get_sqla_query()` flow
therefore receives
inconsistent value types when evaluating `removed_filters` and
`applied_template_filters`
around lines 4039-4043, potentially retaining or mishandling the filter
instead of
treating it as an applied adhoc filter.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5c2ee46b21554914a2c1afe39981e05e&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=5c2ee46b21554914a2c1afe39981e05e&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:** 4028:4036
**Comment:**
*Logic Error: The renamed adhoc column is appended to
`applied_adhoc_filters_columns` as a plain label string, whereas the existing
adhoc path appends the original adhoc column object. Any later logic that
compares or processes this collection as adhoc column definitions will not
recognize the string, which can cause the filter to be retained or processed
incorrectly. Store the original adhoc filter column definition, or update all
consumers to consistently handle labels.
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%2F38716&comment_hash=b921a858e08f82ade83dcddb248c4eae701cc321fde50b491dc77bddc168a82b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38716&comment_hash=b921a858e08f82ade83dcddb248c4eae701cc321fde50b491dc77bddc168a82b&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]
