codeant-ai-for-open-source[bot] commented on code in PR #42598:
URL: https://github.com/apache/superset/pull/42598#discussion_r3698238946
##########
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:
The concern is valid. `do_execute*` only covers statement execution, while
Shillelagh may defer virtual-table reads until the APSW cursor is consumed.
Consequently, the `ContextVar` can be reset before later `get_data` calls,
causing the limit to be reapplied during inner-table rescans.
The flag should remain active for the cursor’s lifetime rather than only for
`super().do_execute*`. A concise fix is to wrap the returned cursor/fetch
iteration and reset the context variable only when the cursor is exhausted or
closed. The wrapper must also preserve the existing token-reset behavior on
execution errors.
The regression coverage should include multiple joined rows, for example:
```sql
SELECT t1.b, t2.b
FROM "database1.table1_large" AS t1
JOIN "database2.table2_late_match" AS t2
ON t1.a = t2.a
```
with `table2_late_match` containing matches for both `a = 2` and `a = 3`,
while `SUPERSET_META_DB_LIMIT = 2`. The expected result must contain both rows;
otherwise the test would not detect a limit being reapplied during later cursor
iteration.
Thus, this comment should be addressed: the current execution-scoped flag is
insufficient for lazy result consumption.
--
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]
