codeant-ai-for-open-source[bot] commented on code in PR #37841:
URL: https://github.com/apache/superset/pull/37841#discussion_r2787467863
##########
superset/sql/parse.py:
##########
@@ -827,7 +827,7 @@ def has_cte(self) -> bool:
:return: True if the statement has a CTE at the top level.
"""
- return "with" in self._parsed.args
+ return "with_" in self._parsed.args
Review Comment:
**Suggestion:** The CTE check relies only on the presence of the "with_" key
in the args dict, but `as_cte` later sets `self._parsed.args["with_"] = None`;
if `as_cte` is called more than once on the same instance, `has_cte()` will
still return True and `self._parsed.args["with_"].expressions` will raise an
AttributeError because the value is None instead of a valid expression. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Reusing SQLStatement.as_cte twice crashes with AttributeError.
- ⚠️ Affects MSSQL CTE wrapping used when building charts.
- ⚠️ Current code treats cleared CTE metadata as still present.
- ⚠️ Suggestion improves robustness of SQL AST transformation.
```
</details>
```suggestion
return bool(self._parsed.args.get("with_"))
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a Python shell in the Superset repo environment and import
`SQLStatement` from
`superset.sql.parse` (implementation at `superset/sql/parse.py:548-924`).
2. Instantiate a statement that contains a top-level CTE, for example:
`stmt = SQLStatement("WITH c AS (SELECT 1 AS x) SELECT * FROM c",
engine="postgresql")`.
3. Call `stmt_cte_1 = stmt.as_cte()` which executes `SQLStatement.as_cte()`
at
`superset/sql/parse.py:832-851`.
- Inside this method, when `self.has_cte()` is True, `existing_ctes =
self._parsed.args["with_"].expressions` is evaluated and then
`self._parsed.args["with_"] = None` is executed.
- After the call returns, `self._parsed.args` still contains the key
`"with_"` but its
value is now `None`.
4. Call `stmt_cte_2 = stmt.as_cte()` again on the same `stmt` instance.
- `has_cte()` at `superset/sql/parse.py:824-830` returns `True` because
it only checks
`"with_" in self._parsed.args`, which is still True even though the value
is `None`.
- `as_cte()` then executes `existing_ctes =
self._parsed.args["with_"].expressions`
(line ~843), attempting to access `.expressions` on `None`, which raises
`AttributeError: 'NoneType' object has no attribute 'expressions'`.
- This crash would propagate to any caller that reuses the same
`SQLStatement` instance
and calls `as_cte()` more than once.
5. No other safeguards or value checks around `self._parsed.args["with_"]`
exist in
`SQLStatement.as_cte()`, so the bug is inherent to the current
implementation; changing
`has_cte()` to check that the value is truthy
(`bool(self._parsed.args.get("with_"))`)
prevents this invalid state from being treated as "has CTE".
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/sql/parse.py
**Line:** 830:830
**Comment:**
*Logic Error: The CTE check relies only on the presence of the "with_"
key in the args dict, but `as_cte` later sets `self._parsed.args["with_"] =
None`; if `as_cte` is called more than once on the same instance, `has_cte()`
will still return True and `self._parsed.args["with_"].expressions` will raise
an AttributeError because the value is None instead of a valid expression.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37841&comment_hash=227449e751f493d74dd69d3ccfdb8a3e0bbf782fdd61148ac6d71465bea6736a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37841&comment_hash=227449e751f493d74dd69d3ccfdb8a3e0bbf782fdd61148ac6d71465bea6736a&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]
