From f27caf53a1d0ea67392b1b4c1092d21df3dccd5d Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Wed, 19 Aug 2026 12:31:59 +1000 Subject: [PATCH] fix(relations,fixtures): ibis-sqlite null date/timestamp binding crash (item 112, IB-DT-19) ibis's SQLite backend stages every in-memory table via a pandas roundtrip (_register_in_memory_table -> op.data.to_frame()); a null date/timestamp value becomes pandas NaT, and stdlib sqlite3 has no adapter for NaTType, raising ProgrammingError before mountainash's own visitor/compile machinery ever runs. Verified empirically (ibis 12.0.0) that explicit PyArrow schemas, ibis.memtable(schema=...), and object-dtype pandas columns all still crash -- ibis re-normalises internally before the pandas roundtrip regardless of input shape. Fix: relations/backends/relation_systems/ibis/_sqlite_compat.py registers a single idempotent sqlite3 adapter (NaT -> NULL), invoked lazily wherever Mountainash builds an ibis.memtable()/create_table() from data that could contain a null temporal value: - Production: relation_visitor.py cross-type join coercion, relsys_ib_ext_ma_util.py resource ingestion (2 call sites). Scope expanded beyond the original test-infra-only backlog item after confirming these crash independent of mountainash-data's own DEBT-13 (mountainash builds ibis.memtable() directly, not through that layer). - Test fixtures: backend_registry.py/backend_helpers.py/conftest.py's 4 duplicated ibis-sqlite table-creation call sites now delegate to one shared helper instead of re-registering the workaround. Rejected: a pandas.DataFrame.to_sql() bypass avoids the crash but silently degrades Boolean columns to Int64 -- an observable regression for every existing non-null fixture. Rejected eager top-level adapter registration at package-import time -- ibis-framework has no unconditional pandas dependency (only specific backend extras do), so it would regress every pandas-free ibis backend user. Tests: - tests/fixtures/test_backend_registry.py, test_backend_helpers.py (new): null date + null datetime regression fixture, parametrized over ALL_BACKENDS, for REGISTRY.build and BackendDataFrameFactory.create/ create_pair. - test_backend_registry.py: xfail(strict=True) upstream-fix monitor -- reproduces the raw crash in a subprocess isolated from mountainash's own process-global adapter patch, so it flips to XPASS(strict) and fails CI the moment ibis fixes IB-DT-19 upstream. - backend_helpers.py::get_count: fixed a pre-existing narwhals-lazy gap (missing .collect()) blocking the new ALL_BACKENDS parametrization. docs/upstream-issue-drafts/IB-DT-19-sqlite-nat-binding.md: drafted, not filed -- ready-to-file ibis issue text for when someone decides to submit it. Backlog: mountainash-central h.backlog/active/ibis-sqlite-fixture-factory-nat-crash.md updated (commit 425f6e7) to record the scope expansion and resolved fix shape. --- .../IB-DT-19-sqlite-nat-binding.md | 111 ++++++++++++++++++ .../relation_systems/ibis/_sqlite_compat.py | 59 ++++++++++ .../relsys_ib_ext_ma_util.py | 5 + .../core/unified_visitor/relation_visitor.py | 4 + tests/conftest.py | 3 +- tests/fixtures/backend_helpers.py | 10 +- tests/fixtures/backend_registry.py | 18 ++- tests/fixtures/test_backend_helpers.py | 50 ++++++++ tests/fixtures/test_backend_registry.py | 88 ++++++++++++++ 9 files changed, 342 insertions(+), 6 deletions(-) create mode 100644 docs/upstream-issue-drafts/IB-DT-19-sqlite-nat-binding.md create mode 100644 src/mountainash/relations/backends/relation_systems/ibis/_sqlite_compat.py create mode 100644 tests/fixtures/test_backend_helpers.py diff --git a/docs/upstream-issue-drafts/IB-DT-19-sqlite-nat-binding.md b/docs/upstream-issue-drafts/IB-DT-19-sqlite-nat-binding.md new file mode 100644 index 00000000..6bd1afa9 --- /dev/null +++ b/docs/upstream-issue-drafts/IB-DT-19-sqlite-nat-binding.md @@ -0,0 +1,111 @@ + + +# Draft: `ibis.sqlite` `create_table`/in-memory registration crashes on a null date/timestamp value (`NaTType` binding error) + +--- + +## Bug + +`Backend.create_table()` (and any expression execution that registers an +in-memory table, e.g. `ibis.memtable(...)` compiled against a SQLite +connection) crashes with `sqlite3.ProgrammingError` when the input contains a +null `date`/`timestamp` value, for **every** input shape I tried — a plain +dict, an explicit-schema PyArrow table, `ibis.memtable(..., schema=...)`, and +an object-dtype pandas column all hit the identical crash. + +### Reproduction + +```python +import ibis +import datetime as dt + +data = { + "id": [1, 2], + "ts": [None, dt.datetime(2024, 1, 1, 12, 0, 0)], +} + +conn = ibis.sqlite.connect(":memory:") +t = conn.create_table("t", data, overwrite=True) +``` + +``` +Traceback (most recent call last): + ... + File ".../ibis/backends/sqlite/__init__.py", line 382, in _register_in_memory_table + cur.executemany(insert_stmt, data) +sqlite3.ProgrammingError: Error binding parameter 2: type 'NaTType' is not supported +``` + +A null `date32` value happens to work — but only incidentally, via PyArrow's +`Table.to_pandas(date_as_object=True)` default converting it to Python `None` +before the pandas roundtrip. A null `timestamp` has no such incidental +escape hatch: PyArrow's `to_pandas()` always maps a null timestamp to +`datetime64[ns]`'s `NaT`, and `sqlite3` has no adapter for `NaTType`. + +### Root cause + +`ibis.backends.sqlite.Backend._register_in_memory_table` always stages the +in-memory table via a pandas roundtrip before binding rows: + +```python +def _register_in_memory_table(self, op: ops.InMemoryTable) -> None: + ... + df = op.data.to_frame() + data = df.itertuples(index=False) + ... + with self.begin() as cur: + cur.execute(create_stmt) + cur.executemany(insert_stmt, data) +``` + +`op.data.to_frame()` always produces a pandas `datetime64` column for a +timestamp field, and pandas has no way to represent a missing value in a +`datetime64` column other than `NaT`. Because the roundtrip happens +internally, no input shape avoids it — I confirmed all of the following still +crash the same way: + +- Explicit-schema `pyarrow.Table` (`pa.schema([("ts", pa.timestamp("us"))])`) +- `ibis.memtable(data, schema=ibis.schema({"ts": "timestamp"}))` +- A pandas `DataFrame` with the timestamp column forced to `dtype=object` + before being passed in (ibis re-normalises it to `datetime64` internally + regardless) + +### Expected behavior + +A null timestamp/date value should round-trip through `create_table`/table +registration the same way it does for every other ibis-supported backend — +binding as SQL `NULL`, not raising `ProgrammingError`. + +### Suggested fix direction + +Register a `sqlite3.register_adapter` for the pandas `NaTType` (or, +upstream, whatever internal NaT-like sentinel `_register_in_memory_table` +produces) that converts it to `None` before binding — this is the smallest +fix, doesn't change any other type's binding behavior, and matches how every +other backend already treats a missing temporal value. + +### Environment + +- ibis: 12.0.0 +- python: 3.12.12 +- pandas: 3.0.5 +- sqlite3 (stdlib): module 2.6.0, libsqlite3 3.50.4 +- platform: macOS-26.4-arm64-arm-64bit + +### Downstream impact (context, not required for the upstream report) + +Found via [mountainash](https://github.com/mountainash-io/mountainash)'s +cross-backend test suite and confirmed to also affect mountainash's own +production code (any relation that cross-type-joins a null-timestamp frame +against, or ingests one into, an ibis-sqlite-backed relation). Worked around +downstream with the `sqlite3.register_adapter` technique described above +(`relations/backends/relation_systems/ibis/_sqlite_compat.py`), tracked +internally as `IB-DT-19` pending this upstream report. diff --git a/src/mountainash/relations/backends/relation_systems/ibis/_sqlite_compat.py b/src/mountainash/relations/backends/relation_systems/ibis/_sqlite_compat.py new file mode 100644 index 00000000..c0926d58 --- /dev/null +++ b/src/mountainash/relations/backends/relation_systems/ibis/_sqlite_compat.py @@ -0,0 +1,59 @@ +"""Compatibility shim for Ibis's SQLite backend. + +Ibis's SQLite backend (``ibis.backends.sqlite.Backend._register_in_memory_table``) +always stages an in-memory table via a pandas roundtrip (``op.data.to_frame()``) +before binding rows through the stdlib ``sqlite3`` module -- regardless of +whether the table was built from a dict, a PyArrow table, or an +``ibis.memtable(..., schema=...)`` call with an explicit temporal schema. A null +``date``/``timestamp`` value becomes pandas ``NaT`` during that roundtrip, and +``sqlite3`` has no adapter for ``NaTType`` -- ``cur.executemany()`` raises +``sqlite3.ProgrammingError("Error binding parameter N: type 'NaTType' is not +supported")`` before Mountainash's own visitor/compile machinery ever runs. + +Verified empirically against ibis 12.0.0 (2026-08-18): explicit PyArrow +schemas, ``ibis.memtable(schema=...)``, and object-dtype pandas columns all +still crash, because Ibis re-normalises the input internally before reaching +the pandas roundtrip. Tracked upstream as ``IB-DT-19`` in +``registry/upstream-issues.yaml`` (status: ``needs_filing`` as of 2026-08-18 -- +no upstream ibis issue exists yet). + +This module registers a single process-global ``sqlite3`` adapter that binds +``NaT`` as ``NULL``, matching how every other backend already treats a missing +temporal value. It does not touch Ibis's own type inference, so schema +fidelity (including ``Boolean`` columns, which a naive ``pandas.to_sql`` +bypass would silently degrade to ``Int64``) is unaffected. +""" +from __future__ import annotations + +_NAT_ADAPTER_INSTALLED = False + + +def ensure_sqlite_nat_adapter() -> None: + """Register a ``sqlite3`` adapter so pandas ``NaT`` binds as ``NULL``. + + Idempotent and process-global: ``sqlite3.register_adapter`` just + overwrites one dict entry, so repeated calls are a cheap no-op. Must be + called before any Ibis-SQLite table is created/executed with data that + might contain a null temporal value -- call sites include every place + Mountainash builds an ``ibis.memtable()``/``create_table()`` from + caller- or file-supplied data that could reach a SQLite-backed + connection (cross-type join coercion, resource ingestion, and the + cross-backend test-fixture factory). + + Silently returns if pandas is not importable: Ibis's own SQLite roundtrip + requires pandas too (``ibis-framework[sqlite]`` depends on it), so if + pandas is missing here, the crash this guards against cannot occur either. + """ + global _NAT_ADAPTER_INSTALLED + if _NAT_ADAPTER_INSTALLED: + return + + import sqlite3 + + try: + import pandas as pd + except ImportError: + return + + sqlite3.register_adapter(type(pd.NaT), lambda _: None) + _NAT_ADAPTER_INSTALLED = True diff --git a/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py b/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py index 7b8b85bc..2f0a8e1d 100644 --- a/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py +++ b/src/mountainash/relations/backends/relation_systems/ibis/extensions_mountainash/relsys_ib_ext_ma_util.py @@ -10,6 +10,9 @@ from mountainash.relations.core.relation_protocols.relation_systems.extensions_mountainash import ( MountainashExtensionRelationSystemProtocol, ) +from mountainash.relations.backends.relation_systems.ibis._sqlite_compat import ( + ensure_sqlite_nat_adapter, +) class MountainashIbisExtensionRelationSystem(MountainashExtensionRelationSystemProtocol[ir.Table]): @@ -130,6 +133,7 @@ def read_resource(self, resource: Any) -> ir.Table: MountainashPolarsExtensionRelationSystem, ) lf = MountainashPolarsExtensionRelationSystem()._read_inline(resource) + ensure_sqlite_nat_adapter() return ibis.memtable(lf.collect().to_arrow()) fmt = self._detect_format_name(resource) @@ -158,6 +162,7 @@ def read_resource(self, resource: Any) -> ir.Table: # Fallback: mountainash-files -> Arrow -> memtable (no pandas). The files # reader honours the full CSV dialect via CsvSpec (>=26.7.1). + ensure_sqlite_nat_adapter() return ibis.memtable(rf.parse_resource_to_arrow(resource)) @staticmethod diff --git a/src/mountainash/relations/core/unified_visitor/relation_visitor.py b/src/mountainash/relations/core/unified_visitor/relation_visitor.py index 72964714..0e49cd57 100644 --- a/src/mountainash/relations/core/unified_visitor/relation_visitor.py +++ b/src/mountainash/relations/core/unified_visitor/relation_visitor.py @@ -657,6 +657,10 @@ def _coerce_to_match(target: Any, value: Any) -> Any: source_type = type(value).__name__ try: import ibis + from mountainash.relations.backends.relation_systems.ibis._sqlite_compat import ( + ensure_sqlite_nat_adapter, + ) + ensure_sqlite_nat_adapter() if is_narwhals_lazyframe(value): value = value.to_native() return ibis.memtable(value) diff --git a/tests/conftest.py b/tests/conftest.py index 32819958..171913b7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -33,6 +33,7 @@ from fixtures.backend_registry import ( REGISTRY as BACKEND_REGISTRY, ALL_BACKENDS, + create_ibis_sqlite_table, ) TEMPORAL_BACKENDS = [ "polars", @@ -239,7 +240,7 @@ def ibis_polars_df(sample_data) -> Any: def ibis_sqlite_df(sample_data) -> Any: """Create Ibis Table with SQLite backend from sample data.""" conn = ibis.sqlite.connect(":memory:") - return conn.create_table("sample", sample_data, overwrite=True) + return create_ibis_sqlite_table(conn, "sample", sample_data) @pytest.fixture diff --git a/tests/fixtures/backend_helpers.py b/tests/fixtures/backend_helpers.py index 32b7cb3b..ec9bf1ab 100644 --- a/tests/fixtures/backend_helpers.py +++ b/tests/fixtures/backend_helpers.py @@ -12,6 +12,8 @@ import narwhals as nw import ibis +from .backend_registry import create_ibis_sqlite_table + class BackendResultHelper: """ @@ -48,7 +50,7 @@ def get_count(df: Any, backend_name: str) -> int: """ if backend_name.startswith("ibis-"): return df.count().execute() - elif backend_name == "polars-lazy": + elif backend_name in ("polars-lazy", "narwhals-lazy"): return df.collect().shape[0] elif backend_name in ["polars", "pandas", "narwhals", "narwhals-polars", "narwhals-pandas"]: return df.shape[0] @@ -299,7 +301,7 @@ def create(data: Dict[str, List], backend_name: str) -> Any: return conn.create_table("test_table", pl_df, overwrite=True) elif backend_name == "ibis-sqlite": conn = ibis.sqlite.connect(":memory:") - return conn.create_table("test_table", data, overwrite=True) + return create_ibis_sqlite_table(conn, "test_table", data) else: raise ValueError(f"Unknown backend: {backend_name}") @@ -340,8 +342,8 @@ def create_pair( return left, right elif backend_name == "ibis-sqlite": conn = ibis.sqlite.connect(":memory:") - left = conn.create_table(left_name, left_data, overwrite=True) - right = conn.create_table(right_name, right_data, overwrite=True) + left = create_ibis_sqlite_table(conn, left_name, left_data) + right = create_ibis_sqlite_table(conn, right_name, right_data) return left, right else: # Non-ibis backends: independent DataFrames, no shared connection needed. diff --git a/tests/fixtures/backend_registry.py b/tests/fixtures/backend_registry.py index cfaa5786..22dd07bd 100644 --- a/tests/fixtures/backend_registry.py +++ b/tests/fixtures/backend_registry.py @@ -68,9 +68,25 @@ def _build_ibis_polars(data: DataDict, table_name: str): return conn.create_table(table_name, pl.DataFrame(data), overwrite=True) +def create_ibis_sqlite_table(conn, name: str, data: DataDict, *, overwrite: bool = True): + """Create an ibis-sqlite table, safe against null date/timestamp values. + + Single source of truth for every ibis-sqlite table-creation call site in + the cross-backend test suite (this module, backend_helpers.py, and + conftest.py's ibis_sqlite_df fixture) -- delegates the NaT-binding + workaround to mountainash's own production fix (item 112 / IB-DT-19) + rather than re-registering a second copy of it. + """ + from mountainash.relations.backends.relation_systems.ibis._sqlite_compat import ( + ensure_sqlite_nat_adapter, + ) + ensure_sqlite_nat_adapter() + return conn.create_table(name, data, overwrite=overwrite) + + def _build_ibis_sqlite(data: DataDict, table_name: str): conn = ibis.sqlite.connect(":memory:") - return conn.create_table(table_name, data, overwrite=True) + return create_ibis_sqlite_table(conn, table_name, data) def _build_ibis_duckdb(data: DataDict, table_name: str): diff --git a/tests/fixtures/test_backend_helpers.py b/tests/fixtures/test_backend_helpers.py new file mode 100644 index 00000000..c0d12517 --- /dev/null +++ b/tests/fixtures/test_backend_helpers.py @@ -0,0 +1,50 @@ +# tests/fixtures/test_backend_helpers.py +"""Self-tests for BackendDataFrameFactory (backend_helpers.py).""" +from __future__ import annotations +import datetime as dt + +import pytest + +from .backend_helpers import BackendDataFrameFactory, BackendResultHelper +from .backend_registry import ALL_BACKENDS + + +# Same shape as test_backend_registry.py's NULL_TEMPORAL_DATA -- mixed +# null/non-null date AND datetime columns. Regression for item 112 / IB-DT-19: +# BackendDataFrameFactory.create/create_pair duplicate the same ibis-sqlite +# construction pattern as the REGISTRY-driven factory and were independently +# confirmed to crash the same way before the fix. +NULL_TEMPORAL_DATA = { + "id": [1, 2, 3], + "when_date": [None, dt.date(2024, 1, 1), dt.date(2024, 1, 2)], + "when_ts": [ + None, + dt.datetime(2024, 1, 1, 12, 0, 0), + dt.datetime(2024, 1, 2, 8, 30, 0), + ], +} + +NULL_TEMPORAL_DATA_RIGHT = { + "id": [1, 2, 3], + "other_date": [dt.date(2024, 2, 1), None, dt.date(2024, 2, 2)], +} + + +@pytest.mark.parametrize("backend_name", ALL_BACKENDS) +def test_create_survives_null_date_and_null_datetime(backend_name): + """BackendDataFrameFactory.create must not crash on a null date/datetime + mixed with non-null rows, for every backend (named explicitly in item 112 + required work #1, alongside _build_ibis_sqlite).""" + df = BackendDataFrameFactory.create(NULL_TEMPORAL_DATA, backend_name) + assert BackendResultHelper.get_count(df, backend_name) == 3 + + +@pytest.mark.parametrize("backend_name", ALL_BACKENDS) +def test_create_pair_survives_null_date_and_null_datetime(backend_name): + """BackendDataFrameFactory.create_pair must not crash when either side of + the pair has a null date/datetime column, for every backend.""" + left, right = BackendDataFrameFactory.create_pair( + NULL_TEMPORAL_DATA, NULL_TEMPORAL_DATA_RIGHT, backend_name + ) + assert BackendResultHelper.get_count(left, backend_name) == 3 + assert BackendResultHelper.get_count(right, backend_name) == 3 diff --git a/tests/fixtures/test_backend_registry.py b/tests/fixtures/test_backend_registry.py index 4cd2995f..1f1e592c 100644 --- a/tests/fixtures/test_backend_registry.py +++ b/tests/fixtures/test_backend_registry.py @@ -1,6 +1,10 @@ # tests/fixtures/test_backend_registry.py """Self-tests for the centralized backend registry.""" from __future__ import annotations +import datetime as dt +import subprocess +import sys +import textwrap import polars as pl import pandas as pd import narwhals as nw @@ -8,10 +12,26 @@ import pytest from .backend_registry import REGISTRY, ALL_BACKENDS, BackendSpec +from .backend_helpers import BackendResultHelper SAMPLE = {"x": [1, 2, 3], "name": ["a", "b", "c"]} +# Regression fixture for item 112 / IB-DT-19: mixed null/non-null date AND +# datetime columns. ibis-sqlite used to crash constructing this shape at all +# (sqlite3.ProgrammingError on the pandas NaT a null value becomes during +# ibis's internal pandas-roundtrip staging) -- before any expression-level +# test, capability gate, or divergence fact ever ran. +NULL_TEMPORAL_DATA = { + "id": [1, 2, 3], + "when_date": [None, dt.date(2024, 1, 1), dt.date(2024, 1, 2)], + "when_ts": [ + None, + dt.datetime(2024, 1, 1, 12, 0, 0), + dt.datetime(2024, 1, 2, 8, 30, 0), + ], +} + def test_registry_has_expected_backends(): expected = { @@ -101,3 +121,71 @@ def test_polars_lazy_ordering_in_all_backends(): # Convention: place polars-lazy right after polars for readability. idx = ALL_BACKENDS.index("polars") assert ALL_BACKENDS[idx + 1] == "polars-lazy" + + +@pytest.mark.parametrize("backend_name", ALL_BACKENDS) +def test_build_survives_null_date_and_null_datetime(backend_name): + """Regression for item 112 / IB-DT-19. + + Every backend must be able to construct a fixture with a null date value + AND a null datetime value mixed with non-null rows -- independent of any + expression-level test. ibis-sqlite is the backend this actually guards: + it used to raise sqlite3.ProgrammingError building this shape at all. + """ + df = REGISTRY[backend_name].build(NULL_TEMPORAL_DATA, table_name="null_temporal") + assert BackendResultHelper.get_count(df, backend_name) == 3 + + +# --- Upstream fix monitor (IB-DT-19) -------------------------------------- +# +# The regression tests above prove Mountainash's OWN construction paths no +# longer crash -- but they do so via _sqlite_compat.ensure_sqlite_nat_adapter(), +# a process-global sqlite3 patch that, once installed anywhere in this test +# session, permanently masks the underlying ibis bug for every subsequent +# test in the same process. To detect when ibis fixes IB-DT-19 upstream, this +# probes bare `ibis.sqlite` in an isolated subprocess that never imports +# mountainash, so the adapter is never installed and ibis's own unpatched +# behaviour is what's actually observed. +_RAW_IBIS_SQLITE_NULL_TEMPORAL_SCRIPT = textwrap.dedent(""" + import datetime as dt + import ibis + + data = { + "id": [1, 2, 3], + "when_date": [None, dt.date(2024, 1, 1), dt.date(2024, 1, 2)], + "when_ts": [ + None, + dt.datetime(2024, 1, 1, 12, 0, 0), + dt.datetime(2024, 1, 2, 8, 30, 0), + ], + } + conn = ibis.sqlite.connect(":memory:") + t = conn.create_table("t", data, overwrite=True) + assert t.count().execute() == 3 + print("OK") +""") + + +@pytest.mark.xfail( + strict=True, + reason=( + "IB-DT-19 (registry/upstream-issues.yaml): raw ibis.sqlite stages " + "in-memory tables via a pandas roundtrip and crashes binding a null " + "date/timestamp value (sqlite3.ProgrammingError on NaTType). Isolated " + "in a bare subprocess -- no mountainash import, so " + "_sqlite_compat.ensure_sqlite_nat_adapter() is never installed -- to " + "observe ibis's own unpatched behaviour. If this XPASSes, ibis fixed " + "the NaT-binding bug upstream: update IB-DT-19 to closed in " + "registry/upstream-issues.yaml and reassess whether " + "ensure_sqlite_nat_adapter() is still needed." + ), +) +def test_raw_ibis_sqlite_null_temporal_upstream_bug_ib_dt_19(): + result = subprocess.run( + [sys.executable, "-c", _RAW_IBIS_SQLITE_NULL_TEMPORAL_SCRIPT], + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, result.stderr + assert "OK" in result.stdout