kokhlo opened a new pull request, #44242:
URL: https://github.com/apache/superset/pull/44242

   ### Summary
   
   Fixes #44241.
   
   `get_columns_description` dispatched the probe statement twice: a direct 
`cursor.execute(mutated_query)` immediately followed by 
`db_engine_spec.execute(cursor, mutated_query, database)`. The engine-spec call 
is the real statement-dispatch point — `BaseEngineSpec.execute` calls 
`cursor.execute(query)` itself (adding arraysize handling, OAuth2 retry mapping 
and DBAPI error normalization), so the first direct execution produced a result 
set that was immediately discarded. Every engine kept this shape: all overrides 
of `execute` (Impala's `execute_async`, Kusto's ARRAY()-unwrapping 
`super().execute`, Hive) still dispatch the statement themselves.
   
   Beyond doubling the probe's cost, the duplicate execution doubled whatever 
per-statement timeout the administrator configured (`max_statement_time` and 
friends): the cap is per statement, and the code decided N=2 — the exact 
scenario measured in the issue (557 s probe under a 300 s statement timeout).
   
   The fix removes the direct `cursor.execute`, leaving a single 
statement-dispatch owner, matching the `sql_lab.py` path noted by the 
`TODO(villebro)` above the block. The `apply_limit_to_sql` wrap-vs-append LIMIT 
question raised in the issue deliberately stays out of scope here — it changes 
the probe query shape for every engine and deserves its own discussion.
   
   ### Testing
   
   Regression asserted at the driver level: a real SQLite connection with 
`set_trace_callback` counts every statement the database actually executes — 
the probe must send exactly one. On the previous code this test fails with `got 
2 executions` of the same statement; with the fix it passes. Two existing 
call-count tests were updated to count `db_engine_spec.execute` (the single 
dispatch point) instead of the removed direct call.
   
   `tests/unit_tests/connectors/sqla/utils_test.py`: 12 passed.
   
   ### Additional context
   
   - Pre-existing unaffected paths verified: the ClickHouse comment-safe retry 
path (`get_column_description_retry_sql`) still executes the original probe 
once and the retry once — unchanged counters reflect exactly that.
   - Verified against current `master` (`superset/connectors/sqla/utils.py` 
line 196) and against all `execute` overrides in `superset/db_engine_specs/`.


-- 
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]

Reply via email to