sadpandajoe opened a new pull request, #45596: URL: https://github.com/apache/superset/pull/45596
### SUMMARY Adds test coverage for what SQL Lab shows when a ClickHouse query fails: the server's own error (code, exception name, offending identifier) should reach `Query.error_message`, `Query.extra["errors"]` and the returned payload, instead of being dropped or replaced by a generic message. Nothing in this area was tested for ClickHouse before; the existing ClickHouse engine-spec tests only cover successful queries. No production code changes. - `tests/unit_tests/sql_lab_clickhouse_errors_test.py` drives `execute_sql_statements` and `get_sql_results` with the real `ClickHouseConnectEngineSpec` and a stand-in DB-API cursor that raises the exception `clickhouse-connect` raises for a server error. It covers a failure at `execute`, a failure while fetching rows, the "Block N out of M" prefix on a multi-statement failure, and the generic-message case below. - `tests/testcontainers/db_engine_specs/test_clickhouse.py` runs failing statements (unknown table, unknown column, syntax error, failing cast, and a runtime error thrown millions of rows into a result) against a real ClickHouse server through `execute_with_cursor`, then checks `extract_error_message` and `extract_errors`. One test is a characterization, not a guarantee of desired behavior: with `show_clickhouse_errors=false` in the connection URI, `clickhouse-connect` itself withholds the server's text and raises only "The ClickHouse server returned an error", so that is all SQL Lab can show. The test pins that current behavior. Note: `clickhouse-connect` reads the full response inside `cursor.execute`, so a failure in the later `fetch_data` step cannot be provoked against a real server; that step is covered only with the stand-in cursor. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable (tests only, no UI change). ### TESTING INSTRUCTIONS Unit tests: `pytest tests/unit_tests/sql_lab_clickhouse_errors_test.py tests/unit_tests/sql_lab_test.py tests/unit_tests/db_engine_specs/test_clickhouse.py` Real-server tests (need Docker for the testcontainers ClickHouse image): `pytest tests/testcontainers/db_engine_specs/test_clickhouse.py -m testcontainers` The author ran the unit tests (204 passed) and ran the testcontainers module against a locally started ClickHouse 26.11.1 server in place of the container (9 passed), because Docker was not available; the container fixture path itself is exercised by CI. Each new test was also confirmed to fail when the error handling in `sql_lab.py`, `sql/execution/utils.py` or the ClickHouse engine spec was deliberately changed to drop the message. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
