codeant-ai-for-open-source[bot] commented on code in PR #41120:
URL: https://github.com/apache/superset/pull/41120#discussion_r3507229636
##########
superset/sql_lab.py:
##########
@@ -433,23 +433,34 @@ def execute_sql_statements( # noqa: C901
db_engine_spec.engine,
set(),
)
- if disallowed_tables and
parsed_script.check_tables_present(disallowed_tables):
- # Report only the tables actually found in the query
- found_tables = set()
- for statement in parsed_script.statements:
- present = {table.table.lower() for table in statement.tables}
- for table in disallowed_tables:
- if table.lower() in present:
- found_tables.add(table)
- raise SupersetDisallowedSQLTableException(found_tables or
disallowed_tables)
+ rls_enabled = is_feature_enabled("RLS_IN_SQLLAB")
Review Comment:
**Suggestion:** Add an explicit type annotation for this new boolean
variable assignment. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new local is a boolean-like value derived from a feature flag and can
be annotated as `bool`, so omitting a type hint violates the rule requiring
type hints on newly added Python variables that can be annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d6a610087db84b6c9b6b5e32118a70c9&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=d6a610087db84b6c9b6b5e32118a70c9&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/sql_lab.py
**Line:** 436:436
**Comment:**
*Custom Rule: Add an explicit type annotation for this new boolean
variable assignment.
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%2F41120&comment_hash=6bc79cab4d441028f124aec7fb9e9a2917c354247c291fffe3ff8ed9f8ec0ef4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41120&comment_hash=6bc79cab4d441028f124aec7fb9e9a2917c354247c291fffe3ff8ed9f8ec0ef4&reaction=dislike'>👎</a>
##########
superset/sql_lab.py:
##########
@@ -433,23 +433,34 @@ def execute_sql_statements( # noqa: C901
db_engine_spec.engine,
set(),
)
- if disallowed_tables and
parsed_script.check_tables_present(disallowed_tables):
- # Report only the tables actually found in the query
- found_tables = set()
- for statement in parsed_script.statements:
- present = {table.table.lower() for table in statement.tables}
- for table in disallowed_tables:
- if table.lower() in present:
- found_tables.add(table)
- raise SupersetDisallowedSQLTableException(found_tables or
disallowed_tables)
+ rls_enabled = is_feature_enabled("RLS_IN_SQLLAB")
+
+ # Resolve the effective per-query schema once and share it between the
+ # denylist check and RLS injection, but only when a control below needs it.
+ # Going through the query-aware ``get_default_schema_for_query`` (rather
than
+ # the static ``get_default_schema``) resolves an unqualified reference to
the
+ # schema the engine actually uses at runtime -- engines without
dynamic-schema
+ # support ignore the request's selected schema -- so both controls match
the
+ # execution path instead of a schema that may never apply.
+ effective_schema = ""
+ if disallowed_tables or rls_enabled:
+ effective_schema = database.get_default_schema_for_query(query)
+
+ if disallowed_tables:
+ # Report only the denylisted tables actually referenced in the query,
+ # honoring schema-qualified entries (e.g.
``information_schema.tables``).
+ found_tables = parsed_script.get_disallowed_tables(
+ disallowed_tables, effective_schema
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation for this new variable
holding parsed denylisted tables. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new intermediate variable is introduced without an explicit type hint.
Since it stores the result of a function call and is a relevant local variable
that can be annotated, the suggestion correctly identifies a type-hint omission.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f172f22fbdad481aa4ee0523eb8daa95&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=f172f22fbdad481aa4ee0523eb8daa95&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/sql_lab.py
**Line:** 452:454
**Comment:**
*Custom Rule: Add an explicit type annotation for this new variable
holding parsed denylisted tables.
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%2F41120&comment_hash=25edd9aa90ca894d2519e00a31600cc19e153b9de12f7150c03c2e0586086246&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41120&comment_hash=25edd9aa90ca894d2519e00a31600cc19e153b9de12f7150c03c2e0586086246&reaction=dislike'>👎</a>
##########
superset/sql_lab.py:
##########
@@ -433,23 +433,34 @@ def execute_sql_statements( # noqa: C901
db_engine_spec.engine,
set(),
)
- if disallowed_tables and
parsed_script.check_tables_present(disallowed_tables):
- # Report only the tables actually found in the query
- found_tables = set()
- for statement in parsed_script.statements:
- present = {table.table.lower() for table in statement.tables}
- for table in disallowed_tables:
- if table.lower() in present:
- found_tables.add(table)
- raise SupersetDisallowedSQLTableException(found_tables or
disallowed_tables)
+ rls_enabled = is_feature_enabled("RLS_IN_SQLLAB")
+
+ # Resolve the effective per-query schema once and share it between the
+ # denylist check and RLS injection, but only when a control below needs it.
+ # Going through the query-aware ``get_default_schema_for_query`` (rather
than
+ # the static ``get_default_schema``) resolves an unqualified reference to
the
+ # schema the engine actually uses at runtime -- engines without
dynamic-schema
+ # support ignore the request's selected schema -- so both controls match
the
+ # execution path instead of a schema that may never apply.
+ effective_schema = ""
Review Comment:
**Suggestion:** Add a concrete type hint for this new schema variable where
it is first declared. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly introduced local variable initialized to a string and later
reused as a schema name, so it is clearly annotatable as `str`; the lack of a
type hint matches the custom rule violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9a2b34d85ecd462690e7e007162d6bf3&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=9a2b34d85ecd462690e7e007162d6bf3&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/sql_lab.py
**Line:** 445:445
**Comment:**
*Custom Rule: Add a concrete type hint for this new schema variable
where it is first declared.
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%2F41120&comment_hash=237fdc4541230310624531f1314428c1a20e2615fc1a0ff4e7f0290b7d0ac94b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41120&comment_hash=237fdc4541230310624531f1314428c1a20e2615fc1a0ff4e7f0290b7d0ac94b&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]
