fix(adapters): resolve DuckDB extension, Arrow NULL-typing, and MSSQL row-format bugs

sqlspec/adapters/duckdb/pool.py

@@ -134,24 +134,52 @@ def _create_connection(self) -> DuckDBConnection:
             force_install = bool(ext_config.get("force_install", False))
             explicit_install = bool(ext_config.get("install", False)) or force_install or bool(install_kwargs)
 
+            install_error: str | None = None
             if explicit_install:
-                self._install_extension_once(connection, ext_name, install_kwargs, force_install, required)
+                install_error = self._install_extension_once(
+                    connection, ext_name, install_kwargs, force_install, required
+                )
 
             try:
                 connection.load_extension(ext_name)
             except Exception as exc:
                 if required:
                     raise
-                log_with_context(
-                    logger,
-                    logging.WARNING,
-                    "pool.extension.load.failed",
-                    adapter=_ADAPTER_NAME,
-                    pool_id=self._pool_id,
-                    database=self._database_name,
-                    extension=ext_name,
-                    error=str(exc),
-                )
+                if explicit_install:
+                    log_with_context(
+                        logger,
+                        logging.WARNING,
+                        "pool.extension.setup.failed",
+                        adapter=_ADAPTER_NAME,
+                        pool_id=self._pool_id,
+                        database=self._database_name,
+                        extension=ext_name,
+                        install_error=install_error,
+                        load_error=str(exc),
+                    )
+                else:
+                    log_with_context(
+                        logger,
+                        logging.WARNING,
+                        "pool.extension.load.failed",
+                        adapter=_ADAPTER_NAME,
+                        pool_id=self._pool_id,
+                        database=self._database_name,
+                        extension=ext_name,
+                        error=str(exc),
+                    )
+            else:
+                if install_error is not None:
+                    log_with_context(
+                        logger,
+                        logging.DEBUG,
+                        "pool.extension.install.failed",
+                        adapter=_ADAPTER_NAME,
+                        pool_id=self._pool_id,
+                        database=self._database_name,
+                        extension=ext_name,
+                        error=install_error,
+                    )
 
         for secret_config in self._secrets:
             _create_secret(connection, secret_config)
@@ -168,8 +196,13 @@ def _install_extension_once(
         install_kwargs: "dict[str, Any]",
         force_install: bool,
         required: bool,
-    ) -> None:
-        """Install an extension once per pool per signature, best-effort unless required."""
+    ) -> "str | None":
+        """Install an extension once per pool per signature, best-effort unless required.
+
+        Returns the install error string on a best-effort failure so the caller can
+        combine it with the load outcome into a single setup result; returns ``None``
+        when the install succeeds or is skipped. Raises when ``required`` is set.
+        """
         signature = (
             ext_name,
             install_kwargs.get("version"),
@@ -178,7 +211,7 @@ def _install_extension_once(
         )
         with self._lock:
             if not force_install and signature in self._installed_signatures:
-                return
+                return None
             try:
                 if force_install:
                     connection.install_extension(ext_name, force_install=True, **install_kwargs)
@@ -189,18 +222,9 @@ def _install_extension_once(
             except Exception as exc:
                 if required:
                     raise
-                log_with_context(
-                    logger,
-                    logging.WARNING,
-                    "pool.extension.install.failed",
-                    adapter=_ADAPTER_NAME,
-                    pool_id=self._pool_id,
-                    database=self._database_name,
-                    extension=ext_name,
-                    error=str(exc),
-                )
-                return
+                return str(exc)
             self._installed_signatures.add(signature)
+            return None
 
     def _apply_extension_flags(self, connection: DuckDBConnection) -> None:
         """Apply connection-level extension flags via SET statements."""

sqlspec/adapters/mssql_python/core.py

@@ -25,7 +25,7 @@
 from sqlspec.utils.type_converters import build_uuid_coercions
 
 if TYPE_CHECKING:
-    from collections.abc import Callable, Mapping
+    from collections.abc import Callable, Mapping, Sequence
     from logging import Logger
 
     from sqlspec.core import StatementConfig
@@ -39,6 +39,7 @@
     "create_mapped_exception",
     "default_statement_config",
     "driver_profile",
+    "materialize_tuple_rows",
 )
 
 _ERROR_NUMBER_PATTERN: Final[re.Pattern[str]] = re.compile(r"\(([-]?\d+)\)")
@@ -118,6 +119,19 @@ def create_mapped_exception(error: Exception, *, logger: "Logger | None" = None)
     return SQLSpecError(f"SQL Server database error. Original error: {error}")
 
 
+def materialize_tuple_rows(fetched: "Sequence[Any] | None") -> "list[tuple[Any, ...]]":
+    """Materialize mssql-python ``Row`` objects into plain tuples.
+
+    ``mssql-python`` returns ``mssql_python.Row`` objects that are iterable and
+    indexable but are not ``tuple`` subclasses. The driver reports
+    ``row_format="tuple"``, so fetched rows are converted to real tuples to keep
+    that contract accurate when results are materialized.
+    """
+    if not fetched:
+        return []
+    return [tuple(row) for row in fetched]
+
+
 def apply_driver_features(
     statement_config: "StatementConfig", driver_features: "Mapping[str, Any] | None"
 ) -> "tuple[StatementConfig, dict[str, Any]]":

sqlspec/adapters/mssql_python/driver.py

@@ -12,7 +12,12 @@
     MssqlPythonRawCursor,
     MssqlPythonSessionContext,
 )
-from sqlspec.adapters.mssql_python.core import create_mapped_exception, default_statement_config, driver_profile
+from sqlspec.adapters.mssql_python.core import (
+    create_mapped_exception,
+    default_statement_config,
+    driver_profile,
+    materialize_tuple_rows,
+)
 from sqlspec.adapters.mssql_python.data_dictionary import MssqlPythonSyncDataDictionary
 from sqlspec.core import (
     build_arrow_result_from_reader,
@@ -162,7 +167,7 @@ def dispatch_execute(self, cursor: "MssqlPythonRawCursor", statement: "SQL") ->
         _execute_cursor(cursor, sql, prepared_parameters)
 
         if statement.returns_rows():
-            fetched = cursor.fetchall()
+            fetched = materialize_tuple_rows(cursor.fetchall())
             column_names = _resolve_column_names(cursor.description, self._column_name_cache)
             return self.create_execution_result(
                 cursor,

sqlspec/utils/arrow_helpers.py

@@ -104,7 +104,7 @@ def convert_dict_to_arrow(
             return batches[0] if batches else pa.RecordBatch.from_pydict({})
 
         return empty_table
-    arrow_table = pa.Table.from_pylist(data)
+    arrow_table = _coerce_null_columns_to_string(pa.Table.from_pylist(data))
 
     if return_format == "reader":
         batches = arrow_table.to_batches(max_chunksize=batch_size)
@@ -120,6 +120,23 @@ def convert_dict_to_arrow(
     return arrow_table
 
 
+def _coerce_null_columns_to_string(table: "ArrowTable") -> "ArrowTable":
+    """Retype value-inferred Arrow ``null`` columns to ``string``.
+
+    ``pa.Table.from_pylist`` infers column types from values only, so a column
+    that is ``NULL`` in every row becomes Arrow ``null`` type and loses the
+    database column's declared type. Downstream consumers (notably DuckDB) then
+    misread such columns, so fall back to ``string`` which safely holds the
+    all-null values without breaking string operations.
+    """
+    import pyarrow as pa
+
+    if not any(pa.types.is_null(field.type) for field in table.schema):
+        return table
+    fields = [field.with_type(pa.string()) if pa.types.is_null(field.type) else field for field in table.schema]
+    return table.cast(pa.schema(fields))
+
+
 def convert_dict_to_arrow_with_schema(
     data: "list[dict[str, Any]]",
     return_format: Literal["table", "reader", "batch", "batches"] = "table",

tests/unit/adapters/test_duckdb/test_extension_lifecycle.py

@@ -159,17 +159,53 @@ def test_load_failure_raises_when_required(monkeypatch: pytest.MonkeyPatch) -> N
         pool._create_connection()
 
 
-def test_install_failure_is_best_effort_by_default(
+def test_install_failure_with_successful_load_records_no_warning(
     monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
 ) -> None:
-    """Test E (install): a failing INSTALL is swallowed with a WARNING by default."""
-    _spy_connect(monkeypatch, fail_install=True)
+    """Test E (setup): install fails but a cached load succeeds => no WARNING (issue #636)."""
+    _spy_connect(monkeypatch, fail_install=True, fail_load=False)
+    pool = DuckDBConnectionPool({"database": ":memory:"}, extensions=[{"name": "postgres", "install": True}])
+
+    with caplog.at_level(logging.DEBUG):
+        pool._create_connection()
+
+    assert [record for record in caplog.records if record.levelno == logging.WARNING] == []
+
+
+def test_install_and_load_failure_reports_single_setup_warning(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test E (setup): install-fail + load-fail collapses to one WARNING carrying both errors (issue #636)."""
+    _spy_connect(monkeypatch, fail_install=True, fail_load=True)
+    pool = DuckDBConnectionPool({"database": ":memory:"}, extensions=[{"name": "postgres", "install": True}])
+
+    with caplog.at_level(logging.WARNING):
+        pool._create_connection()
+
+    warnings = [record for record in caplog.records if record.levelno == logging.WARNING]
+    assert len(warnings) == 1
+    assert warnings[0].message == "pool.extension.setup.failed"
+    fields = warnings[0].__dict__["extra_fields"]
+    assert fields["install_error"] is not None
+    assert fields["load_error"] is not None
+
+
+def test_install_success_with_load_failure_reports_single_setup_warning(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Test E (setup): install-success + load-fail emits one setup WARNING (issue #636)."""
+    _spy_connect(monkeypatch, fail_install=False, fail_load=True)
     pool = DuckDBConnectionPool({"database": ":memory:"}, extensions=[{"name": "postgres", "install": True}])
 
     with caplog.at_level(logging.WARNING):
         pool._create_connection()
 
-    assert any("install" in record.message and record.levelno == logging.WARNING for record in caplog.records)
+    warnings = [record for record in caplog.records if record.levelno == logging.WARNING]
+    assert len(warnings) == 1
+    assert warnings[0].message == "pool.extension.setup.failed"
+    fields = warnings[0].__dict__["extra_fields"]
+    assert fields["install_error"] is None
+    assert fields["load_error"] is not None
 
 
 def test_install_failure_raises_when_required(monkeypatch: pytest.MonkeyPatch) -> None:

tests/unit/adapters/test_mssql_python/test_row_collection.py

@@ -0,0 +1,41 @@
+"""Row-shape fidelity tests for the mssql-python adapter.
+
+``mssql-python`` returns ``mssql_python.Row`` objects that are iterable and
+indexable but are *not* ``tuple`` subclasses. The driver declares
+``row_format="tuple"``, so fetched rows must be materialized into real tuples to
+keep that contract accurate (issue #630).
+"""
+
+from typing import Any
+
+from sqlspec.adapters.mssql_python.core import materialize_tuple_rows
+
+
+class _FakeRow:
+    """Stand-in for ``mssql_python.Row``: iterable/indexable but not a tuple."""
+
+    def __init__(self, values: "tuple[Any, ...]") -> None:
+        self._values = list(values)
+
+    def __iter__(self) -> "Any":
+        return iter(self._values)
+
+    def __getitem__(self, index: int) -> "Any":
+        return self._values[index]
+
+    def __len__(self) -> int:
+        return len(self._values)
+
+
+def test_materialize_tuple_rows_converts_row_objects_to_real_tuples() -> None:
+    rows = [_FakeRow((1, "alice")), _FakeRow((2, "bob"))]
+
+    result = materialize_tuple_rows(rows)
+
+    assert result == [(1, "alice"), (2, "bob")]
+    assert all(type(row) is tuple for row in result)
+
+
+def test_materialize_tuple_rows_handles_empty_and_none() -> None:
+    assert materialize_tuple_rows([]) == []
+    assert materialize_tuple_rows(None) == []

tests/unit/utils/test_arrow_helpers.py

@@ -85,6 +85,31 @@ def test_convert_with_null_values() -> None:
     assert pydict["email"][2] is None
 
 
+def test_all_null_column_falls_back_to_string_type() -> None:
+    """All-NULL columns must not collapse to Arrow ``null`` type (issue #631)."""
+    import pyarrow as pa
+
+    data = [{"id": 1, "label": None}, {"id": 2, "label": None}]
+    result = convert_dict_to_arrow(data, return_format="table")
+
+    label_type = result.schema.field("label").type
+    assert not pa.types.is_null(label_type)
+    assert label_type == pa.string()
+    assert result.schema.field("id").type == pa.int64()
+    assert result.to_pydict()["label"] == [None, None]
+
+
+def test_all_null_column_fallback_applies_to_reader_format() -> None:
+    """The null-type fallback must also flow through non-table return formats."""
+    import pyarrow as pa
+
+    data = [{"label": None}, {"label": None}]
+    reader = convert_dict_to_arrow(data, return_format="reader")
+
+    assert not pa.types.is_null(reader.schema.field("label").type)
+    assert reader.schema.field("label").type == pa.string()
+
+
 def test_convert_with_various_types() -> None:
     """Test converting data with various Python types."""