codeant-ai-for-open-source[bot] commented on code in PR #44908:
URL: https://github.com/apache/superset/pull/44908#discussion_r4171921684
##########
tests/testcontainers/db_engine_specs/test_oracle.py:
##########
@@ -93,3 +97,85 @@ def test_get_columns_maps_native_types(engine: Engine) ->
None:
for col in by_name.values():
spec = OracleEngineSpec.get_column_spec(str(col["type"]))
assert spec is not None
+
+
+def test_cancel_query_stops_a_running_statement(engine: Engine) -> None:
+ """
+ apache/superset#44704 added OracleEngineSpec.get_cancel_query_id/
+ cancel_query, verified only by hand against a real Oracle instance
+ during development (per that PR's own testing notes) -- never pinned
+ down as a repeatable test. This exercises the real pair against a real
+ session: identify the session about to run a long statement on one
+ connection, cancel it from a second connection mid-flight (mirroring
+ SQL Lab's own "new cursor to the db of the query" cancel path), and
+ confirm the first connection's statement actually fails with ORA-01013
+ well before it would complete on its own -- not just that
+ ``cancel_query`` returned ``True`` without effect.
+
+ The container's default user is ``system`` (DBA), which holds the
+ ``ALTER SYSTEM`` privilege ``cancel_query`` needs; see
+ apache/superset#44704's description for the ``False``/ORA-01031 path
+ when that privilege is absent, which this does not exercise.
+ """
+ query_conn = engine.raw_connection()
+ cancel_conn = engine.raw_connection()
+ try:
+ query_cursor = query_conn.cursor()
+ # get_cancel_query_id runs on the same cursor that is about to
+ # execute the query, before it does -- mirrors SQL Lab's own order.
+ cancel_query_id = OracleEngineSpec.get_cancel_query_id(query_cursor,
Query())
+ assert cancel_query_id is not None
+
+ outcome: dict[str, Any] = {}
+
+ def run_slow_query() -> None:
+ try:
+ # A tight PL/SQL loop: CPU-bound with O(1) memory, needs no
+ # privileges beyond CREATE SESSION, and (measured directly
+ # against this same image) ~8s at 500M iterations -- plenty
+ # of margin over the 1.5s delay below. Two things this is
+ # NOT, both tried first:
+ # - `CONNECT BY LEVEL <= n`: materializes the whole
+ # hierarchy and hits ORA-30009 "not enough memory" long
+ # before a count this large finishes -- that failure has
+ # nothing to do with cancellation, so the test would pass
+ # for the wrong reason.
+ # - a loop with no loop-carried side effect (`NULL;` as the
+ # body): optimized away entirely regardless of the
+ # iteration count, so the statement returns instantly and
+ # the cancel never has anything to catch. The `cnt`
+ # assignment here is load-bearing, not cosmetic.
+ query_cursor.execute(
+ "DECLARE cnt NUMBER := 0; BEGIN "
+ "FOR i IN 1..500000000 LOOP cnt := cnt + 1; END LOOP; "
+ "END;"
+ )
+ outcome["completed"] = True
+ except Exception as ex: # noqa: BLE001 # pylint:
disable=broad-except
+ outcome["error"] = ex
+
+ thread = threading.Thread(target=run_slow_query)
+ start = time.monotonic()
+ thread.start()
+ time.sleep(1.5) # let the statement actually start executing
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `07b6cf8`.
Replaced the fixed sleep with polling of `v$session` until the worker’s
Oracle session reports `ACTIVE`, with liveness and timeout guards before
cancellation.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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]
