codeant-ai-for-open-source[bot] commented on code in PR #36747:
URL: https://github.com/apache/superset/pull/36747#discussion_r2632378271
##########
tests/unit_tests/sql/parse_tests.py:
##########
@@ -2891,6 +2891,20 @@ def test_singlestore_engine_mapping():
assert "COUNT(*)" in formatted
+def test_awsathena_engine_mapping():
+ """
+ Test the `awsathena` dialect is properly mapped to ATHENA instead of
PRESTO.
+ """
+ sql = (
+ "USING EXTERNAL FUNCTION my_func(x INT) RETURNS INT LAMBDA
'lambda_name' "
+ "SELECT my_func(id) FROM my_table"
+ )
+ statement = SQLStatement(sql, engine="awsathena")
+
+ # Should parse without errors using Athena dialect
Review Comment:
**Suggestion:** The test calls `statement.format()` but contains no
assertion that the parsed statement is a SELECT (or otherwise valid); add an
explicit assertion like `assert statement.is_select()` to ensure the SQL is
parsed as the expected statement type rather than silently passing without
verification. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Ensure the parsed statement is a SELECT (verifies parsing result)
assert statement.is_select()
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Reasonable and useful: asserting statement.is_select() makes the test
explicit
about the expected kind of parsed AST instead of silently passing because
format() didn't raise. It's a low-risk, high-signal assertion that improves
test quality.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/sql/parse_tests.py
**Line:** 2904:2904
**Comment:**
*Logic Error: The test calls `statement.format()` but contains no
assertion that the parsed statement is a SELECT (or otherwise valid); add an
explicit assertion like `assert statement.is_select()` to ensure the SQL is
parsed as the expected statement type rather than silently passing without
verification.
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>
##########
superset/sql/parse.py:
##########
@@ -58,7 +58,7 @@
SQLGLOT_DIALECTS = {
"base": Dialects.DIALECT,
"ascend": Dialects.HIVE,
- "awsathena": Dialects.PRESTO,
+ "awsathena": Dialects.ATHENA,
Review Comment:
**Suggestion:** Compatibility with older sqlglot versions: directly
referencing `Dialects.ATHENA` may raise an AttributeError if the installed
sqlglot version lacks the ATHENA dialect; use a safe fallback (e.g. PRESTO) via
getattr to avoid runtime attribute errors and ensure parsing still proceeds.
[possible bug]
**Severity Level:** Critical 🚨
```suggestion
"awsathena": getattr(Dialects, "ATHENA", Dialects.PRESTO),
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Using getattr(Dialects, "ATHENA", Dialects.PRESTO) guards against
environments with older sqlglot that lack an ATHENA entry and avoids an
import-time AttributeError. The fallback to PRESTO is a reasonable defensive
choice when ATHENA isn't available.
</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:** 61:61
**Comment:**
*Possible Bug: Compatibility with older sqlglot versions: directly
referencing `Dialects.ATHENA` may raise an AttributeError if the installed
sqlglot version lacks the ATHENA dialect; use a safe fallback (e.g. PRESTO) via
getattr to avoid runtime attribute errors and ensure parsing still proceeds.
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>
--
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]
