bito-code-review[bot] commented on code in PR #34694:
URL: https://github.com/apache/superset/pull/34694#discussion_r2318699656
##########
superset/views/datasource/utils.py:
##########
@@ -44,6 +44,35 @@
return {"row_offset": offset, "row_limit": limit}
+def replace_verbose_with_column(
+ filters: list[dict[str, Any]],
+ columns: Iterable[Any],
+ verbose_attr: str = "verbose_name",
+ column_attr: str = "column_name",
+) -> None:
+ """
+ Replace filter 'col' values that match column verbose_name with the
column_name.
+ Operates in-place on the filters list.
+
+ Args:
+ filters: List of filter dicts, each must have 'col' key.
+ columns: Iterable of column objects with verbose_name and column_name.
+ verbose_attr: Attribute name for verbose/label.
+ column_attr: Attribute name for actual column name.
+ """
+ for f in filters:
+ match = next(
+ (
+ getattr(col, column_attr)
+ for col in columns
+ if getattr(col, verbose_attr) == f["col"]
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing dictionary key validation</b></div>
<div id="fix">
The `f["col"]` access will raise `KeyError` if filter dictionaries don't
have a 'col' key. Add a check for key existence using `f.get("col")` or `"col"
in f` to prevent runtime crashes.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
for f in filters:
+ if "col" not in f:
+ continue
match = next(
(
getattr(col, column_attr)
for col in columns
- if hasattr(col, verbose_attr) and hasattr(col, column_attr)
- and getattr(col, verbose_attr) == f["col"]
+ if hasattr(col, verbose_attr) and hasattr(col, column_attr)
+ and getattr(col, verbose_attr) == f["col"]
),
None,
)
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/34694#issuecomment-3248869498>#cf0842</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/views/datasource/utils.py:
##########
@@ -44,6 +44,35 @@ def get_limit_clause(page: Optional[int], per_page:
Optional[int]) -> dict[str,
return {"row_offset": offset, "row_limit": limit}
+def replace_verbose_with_column(
+ filters: list[dict[str, Any]],
+ columns: Iterable[Any],
+ verbose_attr: str = "verbose_name",
+ column_attr: str = "column_name",
+) -> None:
+ """
+ Replace filter 'col' values that match column verbose_name with the
column_name.
+ Operates in-place on the filters list.
+
+ Args:
+ filters: List of filter dicts, each must have 'col' key.
+ columns: Iterable of column objects with verbose_name and column_name.
+ verbose_attr: Attribute name for verbose/label.
+ column_attr: Attribute name for actual column name.
+ """
+ for f in filters:
+ match = next(
+ (
+ getattr(col, column_attr)
+ for col in columns
+ if getattr(col, verbose_attr) == f["col"]
+ ),
+ None,
+ )
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing attribute existence validation</b></div>
<div id="fix">
The `getattr(col, column_attr)` and `getattr(col, verbose_attr)` calls on
lines 66 and 68 will raise `AttributeError` if column objects don't have the
expected attributes. Add `hasattr()` checks before using `getattr()` to prevent
runtime crashes.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```suggestion
match = next(
(
getattr(col, column_attr)
for col in columns
if hasattr(col, verbose_attr) and hasattr(col, column_attr)
and getattr(col, verbose_attr) == f["col"]
),
None,
)
```
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/34694#issuecomment-3248869498>#cf0842</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
