codeant-ai-for-open-source[bot] commented on code in PR #42598:
URL: https://github.com/apache/superset/pull/42598#discussion_r3693187741
##########
superset/extensions/metadb.py:
##########
@@ -119,6 +134,51 @@ def create_connect_args(self, url: URL) ->
tuple[tuple[()], dict[str, Any]]:
},
)
+ def do_execute(
+ self,
+ cursor: Any,
+ statement: str,
+ parameters: Any,
+ context: Any = None,
+ ) -> None:
+ with self._flag_join_query(statement):
+ super().do_execute(cursor, statement, parameters, context)
Review Comment:
**Suggestion:** The join flag is reset when `super().do_execute` returns,
but APSW/SQLite can defer virtual-table row retrieval until the returned cursor
is iterated. In that case `get_data` runs after this context manager exits,
sees `_executing_join_query` as false, and still applies the per-table limit,
so the regression remains for normal result consumption. Keep the flag active
through cursor iteration or use an execution-scoped mechanism that covers
virtual-table callbacks. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Cross-database joins can still return incomplete results.
- ⚠️ The regression test may fail despite the intended execution wrapper.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e5eaac80bb1441938f367829c952fb13&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=e5eaac80bb1441938f367829c952fb13&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/extensions/metadb.py
**Line:** 144:145
**Comment:**
*Api Mismatch: The join flag is reset when `super().do_execute`
returns, but APSW/SQLite can defer virtual-table row retrieval until the
returned cursor is iterated. In that case `get_data` runs after this context
manager exits, sees `_executing_join_query` as false, and still applies the
per-table limit, so the regression remains for normal result consumption. Keep
the flag active through cursor iteration or use an execution-scoped mechanism
that covers virtual-table callbacks.
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%2F42598&comment_hash=8644bb90b69ffbe4222af02ff4b5c60223fab2e321d3ff334f6de1f7119a3b50&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42598&comment_hash=8644bb90b69ffbe4222af02ff4b5c60223fab2e321d3ff334f6de1f7119a3b50&reaction=dislike'>👎</a>
##########
superset/extensions/metadb.py:
##########
@@ -70,6 +73,18 @@
from superset import db, feature_flag_manager, security_manager
from superset.sql.parse import Table
+# Detects a `JOIN` keyword anywhere in the statement being executed against the
+# `superset://` engine. Shillelagh calls `SupersetShillelaghAdapter.get_data`
once
+# per underlying table, independently of any other table referenced by the same
+# statement, so it has no way on its own to tell whether it's being asked for a
+# standalone table or for one side of a join. `SupersetAPSWDialect.do_execute*`
+# populates `_executing_join_query` for the duration of a statement so that
+# `get_data` can tell the two cases apart (see `get_data` for why this
matters).
+_JOIN_KEYWORD_RE = re.compile(r"\bJOIN\b", re.IGNORECASE)
Review Comment:
**Suggestion:** Detecting joins solely through the literal `JOIN` keyword
misses valid SQLite joins written as comma-separated tables, such as `FROM
table1, table2 WHERE table1.id = table2.id`. Both virtual tables are still
fetched independently, but `_executing_join_query` remains false and the
configured cap can truncate one side before the join, dropping valid results.
Detect multi-table query shapes using SQL parsing or a reliable SQLite
execution signal instead of only matching `JOIN`. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Comma-style cross-database joins can return incomplete results.
- ⚠️ Valid SQLite join syntax bypasses the join-specific limit handling.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=af635e8b1f8341ddab92c0b5a59a00d9&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=af635e8b1f8341ddab92c0b5a59a00d9&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/extensions/metadb.py
**Line:** 83:83
**Comment:**
*Logic Error: Detecting joins solely through the literal `JOIN` keyword
misses valid SQLite joins written as comma-separated tables, such as `FROM
table1, table2 WHERE table1.id = table2.id`. Both virtual tables are still
fetched independently, but `_executing_join_query` remains false and the
configured cap can truncate one side before the join, dropping valid results.
Detect multi-table query shapes using SQL parsing or a reliable SQLite
execution signal instead of only matching `JOIN`.
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%2F42598&comment_hash=1a72d755ddbb3976b005b95010410aafc0d1dc05113ace9cc205d905b84af7fd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42598&comment_hash=1a72d755ddbb3976b005b95010410aafc0d1dc05113ace9cc205d905b84af7fd&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]
