You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/09/29 17:18:54 UTC
(superset) branch master updated: fix(db_engine_specs): don't fail DDL/DML when the statement has no result set (#44714)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 03227878ba3 fix(db_engine_specs): don't fail DDL/DML when the statement has no result set (#44714)
03227878ba3 is described below
commit 03227878ba3598d0d0e76757ca1197b29da06787
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Wed Sep 30 03:18:47 2026 +1000
fix(db_engine_specs): don't fail DDL/DML when the statement has no result set (#44714)
---
.../configuration/configuring-superset.mdx | 10 ++
superset/db_engine_specs/base.py | 8 +-
superset/db_engine_specs/impala.py | 21 ++++
superset/db_engine_specs/mssql.py | 2 -
superset/db_engine_specs/oracle.py | 13 ---
superset/db_engine_specs/postgres.py | 6 --
tests/unit_tests/db_engine_specs/test_base.py | 58 +++++++++++
tests/unit_tests/db_engine_specs/test_impala.py | 109 +++++++++++++++++++++
8 files changed, 205 insertions(+), 22 deletions(-)
diff --git a/docs/admin_docs/configuration/configuring-superset.mdx b/docs/admin_docs/configuration/configuring-superset.mdx
index a615cbf024d..1b6a7f83ff5 100644
--- a/docs/admin_docs/configuration/configuring-superset.mdx
+++ b/docs/admin_docs/configuration/configuring-superset.mdx
@@ -744,6 +744,16 @@ A companion `HASH_ALGORITHM_FALLBACKS` list (default: `["md5"]`) lets UUID looku
This setting affects internal Superset operations only, not user passwords or authentication tokens. Changing it in an existing deployment may invalidate cached values but does not require a database migration.
:::
+## Impala Result Fetching
+
+Statements that produce no result set return no rows without attempting a fetch.
+For Impala, Superset waits for asynchronous execution to complete before checking
+for results, and propagates asynchronous database errors. If an operation is still
+pending when result fetching begins, the additional polling wait is bounded by
+`SQLLAB_TIMEOUT` (in seconds); on expiry, Superset requests cancellation and reports
+a timeout. Configure the driver's network timeout separately to bound individual
+RPC calls.
+
## SQL Lab Query History Pruning
SQL Lab query history is stored in the metadata database and is **not** pruned by default. To trim older rows, enable the `prune_query` Celery beat task by uncommenting it in `CELERY_BEAT_SCHEDULE` and choosing a retention window:
diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index 5deaf1e4d00..da84bdda32b 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -1520,10 +1520,16 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
if cls.arraysize:
cursor.arraysize = cls.arraysize
try:
+ # Statements that return no result set (DDL, DML) leave the cursor
+ # description empty (``None`` per PEP 249), and several DB-API
+ # drivers (mysql-connector, ibm_db, pyexasol, impyla, ...) raise on
+ # a fetch in that state instead of returning no rows.
+ description = cursor.description
+ if not description:
+ return []
if cls.limit_method == LimitMethod.FETCH_MANY and limit:
return cursor.fetchmany(limit)
data = cursor.fetchall()
- description = cursor.description or []
# Create a mapping between column index and a mutator function to normalize
# values with. The first two items in the description row are the column
# name and type.
diff --git a/superset/db_engine_specs/impala.py b/superset/db_engine_specs/impala.py
index b73a169d260..d95167a35fc 100644
--- a/superset/db_engine_specs/impala.py
+++ b/superset/db_engine_specs/impala.py
@@ -128,6 +128,27 @@ class ImpalaEngineSpec(BaseEngineSpec):
except Exception as ex:
raise cls.get_dbapi_mapped_exception(ex) from ex
+ @classmethod
+ def fetch_data(cls, cursor: Any, limit: int | None = None) -> list[tuple[Any, ...]]:
+ """Wait for asynchronous operations using the public cursor API."""
+ if callable(getattr(cursor, "execute_async", None)):
+ from impala.error import Error
+
+ deadline = time.monotonic() + app.config["SQLLAB_TIMEOUT"]
+ try:
+ while cursor.is_executing():
+ if time.monotonic() >= deadline:
+ cursor.cancel_operation()
+ raise TimeoutError("Timed out waiting for the Impala operation")
+ time.sleep(0.1)
+ if cursor.execution_failed():
+ # Fetching surfaces the driver's detailed asynchronous error,
+ # including errors on statements that produce no result set.
+ cursor.fetchall()
+ except Error as ex:
+ raise cls.get_dbapi_mapped_exception(ex) from ex
+ return super().fetch_data(cursor, limit)
+
@classmethod
def handle_cursor(cls, cursor: Any, query: Query) -> None:
"""Stop query and updates progress information"""
diff --git a/superset/db_engine_specs/mssql.py b/superset/db_engine_specs/mssql.py
index 476ea69e3eb..eade36eb107 100644
--- a/superset/db_engine_specs/mssql.py
+++ b/superset/db_engine_specs/mssql.py
@@ -189,8 +189,6 @@ class MssqlEngineSpec(BaseEngineSpec):
def fetch_data(
cls, cursor: Any, limit: Optional[int] = None
) -> list[tuple[Any, ...]]:
- if not cursor.description:
- return []
data = super().fetch_data(cursor, limit)
# Lists of `pyodbc.Row` need to be unpacked further
return cls.pyodbc_rows_to_tuples(data)
diff --git a/superset/db_engine_specs/oracle.py b/superset/db_engine_specs/oracle.py
index 146f221d4c7..e024c42184e 100644
--- a/superset/db_engine_specs/oracle.py
+++ b/superset/db_engine_specs/oracle.py
@@ -90,19 +90,6 @@ class OracleEngineSpec(BaseEngineSpec):
def epoch_ms_to_dttm(cls) -> str:
return "TO_DATE('1970-01-01','YYYY-MM-DD')+(1/24/60/60/1000)*{col}"
- @classmethod
- def fetch_data(
- cls, cursor: Any, limit: Optional[int] = None
- ) -> list[tuple[Any, ...]]:
- """
- :param cursor: Cursor instance
- :param limit: Maximum number of rows to be returned by the cursor
- :return: Result of query
- """
- if not cursor.description:
- return []
- return super().fetch_data(cursor, limit)
-
@classmethod
def get_cancel_query_id(cls, cursor: Any, query: Query) -> Optional[str]:
"""
diff --git a/superset/db_engine_specs/postgres.py b/superset/db_engine_specs/postgres.py
index 6e0daccc51c..6fcd10b85f2 100644
--- a/superset/db_engine_specs/postgres.py
+++ b/superset/db_engine_specs/postgres.py
@@ -288,12 +288,6 @@ class PostgresBaseEngineSpec(BaseEngineSpec):
),
}
- @classmethod
- def fetch_data(cls, cursor: Any, limit: int | None = None) -> list[tuple[Any, ...]]:
- if not cursor.description:
- return []
- return super().fetch_data(cursor, limit)
-
@classmethod
def epoch_to_dttm(cls) -> str:
return "(timestamp 'epoch' + {col} * interval '1 second')"
diff --git a/tests/unit_tests/db_engine_specs/test_base.py b/tests/unit_tests/db_engine_specs/test_base.py
index 7a728c24c91..8c5099cb88e 100644
--- a/tests/unit_tests/db_engine_specs/test_base.py
+++ b/tests/unit_tests/db_engine_specs/test_base.py
@@ -19,6 +19,7 @@
from __future__ import annotations
+import importlib
import json # noqa: TID251
import re
from datetime import timedelta
@@ -1920,6 +1921,63 @@ def test_base_spec_extended_aggregation_func_unknown_name_is_unsupported() -> No
assert BaseEngineSpec.get_extended_aggregation_func("NOT_A_REAL_AGGREGATE") is None
+class _NoResultSetCursor:
+ """
+ A DB-API cursor after a statement that returns no rows (DDL/DML): the
+ description is empty and, like mysql-connector, ibm_db, pyexasol and
+ impyla, fetching raises instead of returning an empty list.
+ """
+
+ def __init__(self, description: list[Any] | None) -> None:
+ self.description = description
+ self.arraysize = 1
+
+ def fetchall(self) -> list[tuple[Any, ...]]:
+ raise RuntimeError("No result set to fetch from")
+
+ def fetchmany(self, size: int | None = None) -> list[tuple[Any, ...]]:
+ raise RuntimeError("No result set to fetch from")
+
+
+@pytest.mark.parametrize(
+ "spec_path",
+ [
+ "superset.db_engine_specs.base.BaseEngineSpec",
+ "superset.db_engine_specs.mysql.MySQLEngineSpec",
+ "superset.db_engine_specs.db2.Db2EngineSpec",
+ "superset.db_engine_specs.exasol.ExasolEngineSpec",
+ "superset.db_engine_specs.impala.ImpalaEngineSpec",
+ "superset.db_engine_specs.postgres.PostgresEngineSpec",
+ "superset.db_engine_specs.oracle.OracleEngineSpec",
+ "superset.db_engine_specs.mssql.MssqlEngineSpec",
+ ],
+)
+@pytest.mark.parametrize("description", [None, []])
+@pytest.mark.parametrize("limit", [None, 100])
+def test_fetch_data_no_result_set(
+ spec_path: str, description: list[Any] | None, limit: int | None
+) -> None:
+ """
+ Statements without a result set return no rows instead of failing after
+ they have already executed.
+ """
+ module_name, class_name = spec_path.rsplit(".", 1)
+ spec = getattr(importlib.import_module(module_name), class_name)
+
+ assert spec.fetch_data(_NoResultSetCursor(description), limit) == []
+
+
+def test_fetch_data_with_result_set(mocker: MockerFixture) -> None:
+ """
+ Rows are still fetched when the cursor describes a result set.
+ """
+ cursor = mocker.MagicMock()
+ cursor.description = [("a", "INTEGER", None, None, None, None, True)]
+ cursor.fetchall.return_value = [(1,), (2,)]
+
+ assert BaseEngineSpec.fetch_data(cursor) == [(1,), (2,)]
+
+
class _EpochSpec(BaseEngineSpec):
"""Minimal spec implementing only ``epoch_to_dttm``, like most engines."""
diff --git a/tests/unit_tests/db_engine_specs/test_impala.py b/tests/unit_tests/db_engine_specs/test_impala.py
index 7e26a5fb24a..eb443ea9bea 100644
--- a/tests/unit_tests/db_engine_specs/test_impala.py
+++ b/tests/unit_tests/db_engine_specs/test_impala.py
@@ -20,6 +20,7 @@ from typing import Optional
from unittest.mock import Mock, patch
import pytest
+from pytest_mock import MockerFixture
from superset.db_engine_specs.impala import ImpalaEngineSpec as spec # noqa: N813
from superset.models.core import Database
@@ -172,3 +173,111 @@ def test_cancel_query_allows_internal_host_with_opt_out(
allow_redirects=False,
)
assert result is True
+
+
+class _DriverError(Exception):
+ """Represent impyla's optional DB-API error base in unit tests."""
+
+
+@pytest.fixture
+def impala_errors(mocker: MockerFixture) -> None:
+ """Provide driver errors without requiring the optional impyla package."""
+ mocker.patch.dict("sys.modules", {"impala.error": Mock(Error=_DriverError)})
+
+
+class _AsyncCursor:
+ """Public impyla API for a pending statement without rows."""
+
+ description: None = None
+
+ def __init__(self, error: Exception | None = None) -> None:
+ """Initialize the pending operation and its optional failure."""
+ self.error: Exception | None = error
+ self.waited: bool = False
+
+ def execute_async(self, query: str) -> None:
+ """Expose the asynchronous execution capability."""
+
+ def is_executing(self) -> bool:
+ """Report one pending poll before completing."""
+ if not self.waited:
+ self.waited = True
+ return True
+ return False
+
+ def execution_failed(self) -> bool:
+ """Report whether the completed operation failed."""
+ return self.error is not None
+
+ def fetchall(self) -> list[tuple[int]]:
+ """Surface asynchronous failures without accepting no-result fetches."""
+ if self.error:
+ raise self.error
+ raise RuntimeError("Trying to fetch results on an operation with no results")
+
+
+def test_fetch_data_waits_for_operation(app_context: None, impala_errors: None) -> None:
+ """DML still pending after execute_async completes before fetch_data returns."""
+ cursor = _AsyncCursor()
+ assert spec.fetch_data(cursor) == []
+ assert cursor.waited
+
+
+def test_fetch_data_raises_operation_error(
+ app_context: None, impala_errors: None
+) -> None:
+ """An asynchronous driver failure is not reported as an empty result."""
+ cursor = _AsyncCursor(_DriverError("AnalysisException: boom"))
+ with pytest.raises(_DriverError, match="AnalysisException: boom"):
+ spec.fetch_data(cursor)
+
+
+@patch("superset.db_engine_specs.impala.time.sleep")
+@patch("superset.db_engine_specs.impala.time.monotonic", side_effect=[0, 1, 31])
+def test_fetch_data_timeout(
+ monotonic_mock: Mock, sleep_mock: Mock, app_context: None, impala_errors: None
+) -> None:
+ """A perpetually pending operation is cancelled within the query timeout."""
+ from flask import current_app
+
+ cursor = Mock()
+ cursor.is_executing.return_value = True
+ with patch.dict(current_app.config, SQLLAB_TIMEOUT=30):
+ with pytest.raises(TimeoutError, match="Timed out waiting"):
+ spec.fetch_data(cursor)
+ cursor.cancel_operation.assert_called_once_with()
+ cursor.fetchall.assert_not_called()
+ sleep_mock.assert_called_once_with(0.1)
+ assert monotonic_mock.call_count == 3
+
+
+@pytest.mark.parametrize("error", [TypeError("bug"), AttributeError("bug")])
+@patch.object(spec, "get_dbapi_mapped_exception")
+def test_fetch_data_preserves_programming_errors(
+ mapped_exception: Mock, error: Exception, app_context: None, impala_errors: None
+) -> None:
+ """Programming errors are not rewritten as database errors."""
+ cursor = Mock()
+ cursor.is_executing.side_effect = error
+ with pytest.raises(type(error), match="bug"):
+ spec.fetch_data(cursor)
+ mapped_exception.assert_not_called()
+
+
+def test_fetch_data_synchronous_cursor() -> None:
+ """A synchronous DB-API cursor needs no asynchronous wait or driver import."""
+ cursor = Mock(spec=["description", "fetchall"])
+ cursor.description = [("a", "INTEGER", None, None, None, None, True)]
+ cursor.fetchall.return_value = [(1,)]
+ assert spec.fetch_data(cursor) == [(1,)]
+
+
+def test_fetch_data_async_result_set(app_context: None, impala_errors: None) -> None:
+ """Completed asynchronous SELECT statements still return their rows."""
+ cursor = Mock()
+ cursor.is_executing.return_value = False
+ cursor.execution_failed.return_value = False
+ cursor.description = [("a", "INTEGER", None, None, None, None, True)]
+ cursor.fetchall.return_value = [(1,)]
+ assert spec.fetch_data(cursor) == [(1,)]
+ cursor.cancel_operation.assert_not_called()