codeant-ai-for-open-source[bot] commented on code in PR #43942:
URL: https://github.com/apache/superset/pull/43942#discussion_r3944788584
##########
superset/daos/query.py:
##########
@@ -78,6 +78,25 @@ def stop_query(client_id: str) -> None:
)
return
+ # An async query runs as a GTF task (superset.sql_lab, keyed by
client_id).
+ # Cancel through GTF so stopping from SQL Lab and from the Task List
view are
+ # the same operation: the task's abort handler kills the warehouse
query and
+ # the task mirrors STOPPED onto the Query row.
+ from superset.commands.tasks.cancel import CancelTaskCommand
+ from superset.daos.tasks import TaskDAO
+ from superset.tasks.sql_queries import SQL_LAB_TASK
+
+ user_id = get_user_id()
+ task = (
+ TaskDAO.find_by_task_key(SQL_LAB_TASK, client_id, "private",
user_id)
+ if user_id is not None
+ else None
+ )
+ if task is not None:
+ CancelTaskCommand(task.uuid).run()
+ return
Review Comment:
**Suggestion:** When the GTF task is still pending, cancellation marks only
the task aborted. This returns without changing the `Query` row, which remains
pending forever instead of becoming stopped. [incomplete implementation]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=321d6d6adc9447bfa5334e63ae136ae1&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=321d6d6adc9447bfa5334e63ae136ae1&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/daos/query.py
**Line:** 89:97
**Comment:**
*Incomplete Implementation: When the GTF task is still pending,
cancellation marks only the task aborted. This returns without changing the
`Query` row, which remains pending forever instead of becoming stopped.
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%2F43942&comment_hash=fe5828e13253b429bb1f112908971f9a6db486448a7e38035cd4ade87bef7f4b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43942&comment_hash=fe5828e13253b429bb1f112908971f9a6db486448a7e38035cd4ade87bef7f4b&reaction=dislike'>๐</a>
##########
superset/commands/sql_lab/execute.py:
##########
@@ -164,13 +172,144 @@ def _run_sql_json_exec_from_scratch(self) ->
SqlJsonExecutionStatus:
self._query_dao.update(
query, {"limit": self._execution_context.query.limit}
)
- return self._sql_json_executor.execute(
- self._execution_context, rendered_query, self._log_params
- )
+ return self._execute(rendered_query)
except Exception:
self._query_dao.update(query, {"status": QueryStatus.FAILED})
raise
+ def _execute(self, rendered_query: str) -> SqlJsonExecutionStatus:
+ """Dispatch to synchronous (inline) or asynchronous (GTF task)
execution.
+
+ Sync runs the query in-process via the unified SQL-Lab executor entry.
+ Async only *prepares* here (the ``Query`` row is committed by this
+ command's transaction); the actual GTF task is scheduled by
+ ``submit_async``, which the endpoint calls after the transaction
commits
+ because scheduling a task cannot run inside an outer ``@transaction``.
+ """
+ if self._execution_context.is_run_asynchronous():
+ return self._prepare_async(rendered_query)
+ return self._execute_sync(rendered_query)
+
+ def _execute_sync(self, rendered_query: str) -> SqlJsonExecutionStatus:
+ """Run the query inline via ``execute_sql_lab_query`` and set the
result."""
+ from superset.sql.execution.sqllab_executor import
execute_sql_lab_query
+ from superset.sql_lab import get_query, handle_query_error
+
+ context = self._execution_context
+ query = context.query
+ timeout = app.config["SQLLAB_TIMEOUT"]
+ store_results = (
+ is_feature_enabled("SQLLAB_BACKEND_PERSISTENCE")
+ and not context.select_as_cta
+ )
+ try:
+ with utils.timeout(
+ seconds=timeout,
+ error_message=f"The query exceeded the {timeout} seconds
timeout.",
+ ):
+ try:
+ data = execute_sql_lab_query(
+ query,
+ rendered_query,
+ return_results=True,
+ store_results=store_results,
+ expand_data=context.expand_data,
+ log_params=self._log_params,
+ )
+ except Exception as ex: # pylint: disable=broad-except
+ # Re-fetch (the session may be poisoned) and build the
error
+ # payload, mirroring the classic synchronous path.
+ data = handle_query_error(ex, get_query(query_id=query.id))
+ except SupersetTimeoutException:
+ raise
+ except Exception as ex:
+ logger.exception("Query %i failed unexpectedly", query.id)
+ raise SupersetGenericDBErrorException(
+ utils.error_msg_from_exception(ex)
+ ) from ex
+
+ context.set_execution_result(data)
+ if data and data.get("status") == QueryStatus.FAILED:
+ if data.get("errors"):
+ errors = [SupersetError(**params) for params in data["errors"]]
+ status = (
+ 500
+ if any(error.level == ErrorLevel.ERROR for error in errors)
+ else 400
+ )
+ raise SupersetErrorsException(errors, status=status)
+ raise SupersetGenericDBErrorException(data["error"])
+ return SqlJsonExecutionStatus.HAS_RESULTS
Review Comment:
**Suggestion:** The executor can return a `STOPPED` payload, but this always
returns `HAS_RESULTS`, causing stopped queries to be reported as successful
results. [logic error]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c4510d7cf9734ccd87237179c6411226&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=c4510d7cf9734ccd87237179c6411226&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/commands/sql_lab/execute.py
**Line:** 231:242
**Comment:**
*Logic Error: The executor can return a `STOPPED` payload, but this
always returns `HAS_RESULTS`, causing stopped queries to be reported as
successful results.
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%2F43942&comment_hash=71b3c6bdd446432c2d57d81a54cdf01d3635f1da7e819898cf0006a2887443ec&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43942&comment_hash=71b3c6bdd446432c2d57d81a54cdf01d3635f1da7e819898cf0006a2887443ec&reaction=dislike'>๐</a>
##########
superset/commands/sql_lab/execute.py:
##########
@@ -164,13 +172,144 @@ def _run_sql_json_exec_from_scratch(self) ->
SqlJsonExecutionStatus:
self._query_dao.update(
query, {"limit": self._execution_context.query.limit}
)
- return self._sql_json_executor.execute(
- self._execution_context, rendered_query, self._log_params
- )
+ return self._execute(rendered_query)
except Exception:
self._query_dao.update(query, {"status": QueryStatus.FAILED})
raise
+ def _execute(self, rendered_query: str) -> SqlJsonExecutionStatus:
+ """Dispatch to synchronous (inline) or asynchronous (GTF task)
execution.
+
+ Sync runs the query in-process via the unified SQL-Lab executor entry.
+ Async only *prepares* here (the ``Query`` row is committed by this
+ command's transaction); the actual GTF task is scheduled by
+ ``submit_async``, which the endpoint calls after the transaction
commits
+ because scheduling a task cannot run inside an outer ``@transaction``.
+ """
+ if self._execution_context.is_run_asynchronous():
+ return self._prepare_async(rendered_query)
+ return self._execute_sync(rendered_query)
+
+ def _execute_sync(self, rendered_query: str) -> SqlJsonExecutionStatus:
+ """Run the query inline via ``execute_sql_lab_query`` and set the
result."""
+ from superset.sql.execution.sqllab_executor import
execute_sql_lab_query
+ from superset.sql_lab import get_query, handle_query_error
+
+ context = self._execution_context
+ query = context.query
+ timeout = app.config["SQLLAB_TIMEOUT"]
+ store_results = (
+ is_feature_enabled("SQLLAB_BACKEND_PERSISTENCE")
+ and not context.select_as_cta
+ )
+ try:
+ with utils.timeout(
+ seconds=timeout,
+ error_message=f"The query exceeded the {timeout} seconds
timeout.",
+ ):
+ try:
+ data = execute_sql_lab_query(
+ query,
+ rendered_query,
+ return_results=True,
+ store_results=store_results,
+ expand_data=context.expand_data,
+ log_params=self._log_params,
+ )
+ except Exception as ex: # pylint: disable=broad-except
+ # Re-fetch (the session may be poisoned) and build the
error
+ # payload, mirroring the classic synchronous path.
+ data = handle_query_error(ex, get_query(query_id=query.id))
+ except SupersetTimeoutException:
+ raise
+ except Exception as ex:
+ logger.exception("Query %i failed unexpectedly", query.id)
+ raise SupersetGenericDBErrorException(
+ utils.error_msg_from_exception(ex)
+ ) from ex
+
+ context.set_execution_result(data)
+ if data and data.get("status") == QueryStatus.FAILED:
+ if data.get("errors"):
+ errors = [SupersetError(**params) for params in data["errors"]]
+ status = (
+ 500
+ if any(error.level == ErrorLevel.ERROR for error in errors)
+ else 400
+ )
+ raise SupersetErrorsException(errors, status=status)
+ raise SupersetGenericDBErrorException(data["error"])
+ return SqlJsonExecutionStatus.HAS_RESULTS
+
+ def _prepare_async(self, rendered_query: str) -> SqlJsonExecutionStatus:
+ """Validate async prerequisites and defer scheduling to
``submit_async``.
+
+ Async SQL Lab execution runs as a GTF task, so it requires the
+ ``GLOBAL_TASK_FRAMEWORK`` feature flag; fail fast with a clear error
when
+ it is disabled rather than surfacing a raw framework error at schedule
time.
+ """
+ if not is_feature_enabled("GLOBAL_TASK_FRAMEWORK"):
+ error = SupersetError(
+ message=__(
+ "Asynchronous SQL Lab execution requires the "
+ "GLOBAL_TASK_FRAMEWORK feature flag to be enabled."
+ ),
+ error_type=SupersetErrorType.ASYNC_WORKERS_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ self._fail_query(error)
+ raise SupersetErrorException(error)
+ self._rendered_query = rendered_query
+ self._pending_async = True
+ return SqlJsonExecutionStatus.QUERY_IS_RUNNING
+
+ def submit_async(self) -> None:
+ """Schedule the async GTF SQL task, outside this command's transaction.
+
+ Called by the endpoint after ``run`` commits. Scheduling a GTF task
+ acquires its own lock/transaction and refuses to run inside an outer
+ ``@transaction`` (see ``SubmitTaskCommand``), so it must happen here.
+ """
+ if not self._pending_async:
+ return
+ from superset_core.tasks.types import TaskOptions
+
+ from superset.tasks.sql_queries import run_sql_lab_query
+
+ context = self._execution_context
+ query = context.query
+ try:
+ run_sql_lab_query.schedule(
+ query.id,
+ self._rendered_query,
+ store_results=not context.select_as_cta,
+ expand_data=context.expand_data,
+ username=get_username(),
+ start_time=now_as_float(),
+ log_params=self._log_params,
+ # PRIVATE dedup on the browser-generated client_id subsumes the
+ # classic ``is_query_handled`` idempotency guard.
+ options=TaskOptions(task_key=query.client_id),
+ )
Review Comment:
**Suggestion:** Async queries always persist results, even when
`SQLLAB_BACKEND_PERSISTENCE` is disabled, unlike synchronous queries. This
changes configuration behavior and can require an unavailable results backend.
[api mismatch]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=aa8d702f58f54af498d3dae0c004cb66&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=aa8d702f58f54af498d3dae0c004cb66&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/commands/sql_lab/execute.py
**Line:** 282:293
**Comment:**
*Api Mismatch: Async queries always persist results, even when
`SQLLAB_BACKEND_PERSISTENCE` is disabled, unlike synchronous queries. This
changes configuration behavior and can require an unavailable results backend.
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%2F43942&comment_hash=aebebdc130f1327925454923009dfa7a6dd272dfb4d0ba8c0aeebc80355d728e&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43942&comment_hash=aebebdc130f1327925454923009dfa7a6dd272dfb4d0ba8c0aeebc80355d728e&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]
