codeant-ai-for-open-source[bot] commented on code in PR #43641:
URL: https://github.com/apache/superset/pull/43641#discussion_r3878579959
##########
superset/sql/execution/executor.py:
##########
@@ -817,14 +817,9 @@ def _check_disallowed_functions(self, script: SQLScript)
-> set[str] | None:
if not engine_disallowed:
return None
- # Check each statement for disallowed functions
- found = set()
- for statement in script.statements:
- # Use the statement's AST to check for function calls
- statement_str = str(statement).upper()
- for func in engine_disallowed:
- if func.upper() in statement_str:
- found.add(func)
+ found = {
+ func for func in engine_disallowed if
script.check_functions_present({func})
+ }
Review Comment:
**Suggestion:** Calling `script.check_functions_present` once for every
configured function repeatedly walks the entire AST and rebuilds the
function-name set. For the default PostgreSQL denylist, every query without a
match performs dozens of redundant AST traversals, adding avoidable CPU and
latency to every execution. Compute the present function names once or expose a
matcher that returns the intersection with the denylist in a single traversal.
[performance]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ SQL validation repeats full AST walks for multi-function denylists.
- โ ๏ธ Query latency and CPU usage increase for configured engines.
- โ ๏ธ Overhead occurs before synchronous and asynchronous execution paths.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7644ba3edb9c4c039b6e109c9b612488&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=7644ba3edb9c4c039b6e109c9b612488&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/sql/execution/executor.py
**Line:** 820:822
**Comment:**
*Performance: Calling `script.check_functions_present` once for every
configured function repeatedly walks the entire AST and rebuilds the
function-name set. For the default PostgreSQL denylist, every query without a
match performs dozens of redundant AST traversals, adding avoidable CPU and
latency to every execution. Compute the present function names once or expose a
matcher that returns the intersection with the denylist in a single traversal.
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%2F43641&comment_hash=7c64ce37190f2460cad1cbdc4f1fbd159b728f36f811ef50603d255f0bb450dc&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43641&comment_hash=7c64ce37190f2460cad1cbdc4f1fbd159b728f36f811ef50603d255f0bb450dc&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]
