From 13217ca751254a6a29e1c293916dfd4d12f44eb7 Mon Sep 17 00:00:00 2001 From: Nathaniel Ramm Date: Tue, 18 Aug 2026 10:44:19 +1000 Subject: [PATCH] fix(relations): order Ibis join_asof result deterministically (IB-REL-11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause (PR #297 CI investigation): DuckDB gives no row-order guarantee for ASOF JOIN output absent an explicit ORDER BY. Ibis's join_asof backend (relsys_ib_ext_ma_util.py) never applied one, so test_asof_backward_strategy[ibis-duckdb] flipped strict-xfail/XPASS ~50/50 across independent runs (confirmed via 8 isolated process reruns pre/post fix). Values were always correct; only row order backing the test's positional assertions flapped. Same incident class as backlog item 101 (PR #293, 2026-08-16, flagged but not fixed then). Fix: order the join_asof result by the join key(s) before returning, matching polars/narwhals' left-order preservation for asof joins. Deterministic across 8/8 verification runs post-fix. IB-REL-11 split: ibis-duckdb portion closed resolved_in_mountainash; distinct, still-genuine ibis-sqlite raise (no ASOF JOIN translation) carved out to new IB-REL-13. Updated divergences.py, upstream-issues.yaml, known-divergences.md, and the test's xfail marker in lockstep; regenerated expression-coverage.{md,json} and _spine_expectation_census.md. AGENTS.md: removed the stale 'local Ibis fork' dependency note — CI/dev both resolve ibis-framework from PyPI, no path dependency exists. Backlog: filed mountainash-central item 108 for a distinct, unrelated latent bug found in the same function (join_asof silently drops the strategy= parameter on Ibis — forward/nearest behave as backward) and closed item 101's flagged sub-issue. --- AGENTS.md | 8 +--- docs/known-divergences.md | 30 ++++++++---- docs/reference/expression-coverage.json | 25 +++++----- docs/reference/expression-coverage.md | 2 +- registry/upstream-issues.yaml | 46 ++++++++++++++----- .../core/capabilities/divergences.py | 14 +++--- .../relsys_ib_ext_ma_util.py | 10 +++- tests/_spine_expectation_census.md | 2 +- .../cross_backend/test_rel_join_results.py | 2 +- 9 files changed, 89 insertions(+), 50 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 53467c5c..386f80d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -195,13 +195,7 @@ For detailed file organisation see principle: `f.development-practices/file-orga ## Dependencies -**IMPORTANT:** Using **local Ibis fork** with Polars calendar interval fix: - -```toml -ibis-framework = { path = "/home/nathanielramm/git/ibis", extras = ["pandas", "sqlite", "duckdb"] } -``` - -All other dependencies are in `pyproject.toml`. +All dependencies are declared in `pyproject.toml` (`ibis-framework>=9.0.0` resolved from PyPI for the `ibis` extra — no local fork or path dependency). **Workspace dependency for DataPackage I/O:** `mountainash-utils-files` (sibling package, optional `storage` extra) provides `StorageFacade` used by `core/io.py` to load remote `DataResource` paths. `core.io.is_remote()` delegates to the facade's scheme registry for auto-detection; `core.io.facade_read_bytes()` calls `StorageFacade.from_path()`. Local paths bypass the facade and use Polars directly. The import is lazy so a local-only test run never touches `mountainash_utils_files`. diff --git a/docs/known-divergences.md b/docs/known-divergences.md index bad7993e..85dbecb6 100644 --- a/docs/known-divergences.md +++ b/docs/known-divergences.md @@ -120,8 +120,9 @@ Total divergences tracked: **125** | IB-REL-08 | Ibis backend rename mapping inverted — expects {new: old} not {old: new} | ibis-duckdb, ibis-polars, ibis-sqlite | mountainash_internal | Strict xfail | Investigating | | IB-REL-09 | Ibis backend passes unsupported suffixes kwarg to Table.join() | ibis-duckdb, ibis-polars, ibis-sqlite | mountainash_internal | Strict xfail | Investigating | | IB-REL-10 | ibis-sqlite: drop_nans/unpivot/melt raise OperationNotDefinedError | ibis-sqlite | upstream_feature_gap | Strict xfail | Needs Filing | -| IB-REL-11 | asof join is unreliable on Ibis SQL backends | ibis-duckdb, ibis-sqlite | upstream_bug | Strict xfail | Investigating | +| IB-REL-11 | RESOLVED: ibis-duckdb asof join returned nondeterministic row order | ibis-duckdb | mountainash_internal | None | Resolved (internal) | | IB-REL-12 | cross_join between two same-named tables from different Ibis connections raises a column-binding error on SQL backends | ibis-duckdb, ibis-sqlite | mountainash_internal | Strict xfail | Investigating | +| IB-REL-13 | ibis-sqlite: asof join raises UnsupportedOperationError | ibis-sqlite | upstream_feature_gap | Strict xfail | Needs Filing | ### Aggregate Operations @@ -1263,18 +1264,18 @@ Total divergences tracked: **125** | Notes | ibis-sqlite lacks array/pivot relational translations: Relation.drop_nans()/ unpivot()/melt() raise OperationNotDefinedError; ibis-duckdb/ibis-polars and polars/narwhals compute them. Renamed from a colliding DivergenceFact id (was "IB-REL-07", which already named an unrelated entry above — the ArrayValue.sort() descending-parameter gap) when backlog item 87's id-collision detector caught the reuse; the DivergenceFact and this entry now self-reference via upstream_ref (backlog item 87, 2026-08-13). | | Last Verified | 2026-08-13 | -### IB-REL-11: asof join is unreliable on Ibis SQL backends +### IB-REL-11: RESOLVED — ibis-duckdb asof join returned nondeterministic row order | Field | Value | |---|---| | Project | ibis | | Category | Relational Operations | -| Root Cause | upstream_bug | -| Affected Backends | ibis-duckdb, ibis-sqlite | -| Status | Investigating | -| Workaround | Strict xfail | -| Notes | Relation.join_asof() returns a diverging (wrong) result on ibis-duckdb and raises UnsupportedOperationError on ibis-sqlite; polars/narwhals compute it correctly. Renamed from a colliding DivergenceFact id (was "IB-REL-08", which already named an unrelated entry above — the backend rename-mapping-inverted gap) when backlog item 87's id-collision detector caught the reuse; the DivergenceFact and this entry now self-reference via upstream_ref (backlog item 87, 2026-08-13). | -| Last Verified | 2026-08-13 | +| Root Cause | mountainash_internal | +| Affected Backends | ibis-duckdb | +| Status | Resolved (internal) | +| Workaround | None | +| Notes | Originally described as "asof join is unreliable on Ibis SQL backends" covering both ibis-duckdb and ibis-sqlite. Root-caused 2026-08-18 (PR #297 CI investigation, repeated single-test process runs flipped xfail/XPASS ~50/50): Relation.join_asof() on the Ibis backend never applied an explicit ORDER BY, and DuckDB gives no row-order guarantee for ASOF JOIN output absent one — the query planner nondeterministically returned rows in original or shuffled order run-to-run. This was not a value divergence: join values were always correct, only the row order backing the test's positional assertions flapped. Fixed by ordering the Ibis join_asof result by the join key(s) (relsys_ib_ext_ma_util.py) to match polars/narwhals' left-order preservation. The distinct ibis-sqlite raise (no ASOF JOIN translation) was split off to IB-REL-13, which remains open. | +| Last Verified | 2026-08-18 | ### IB-REL-12: cross_join between two same-named tables from different Ibis connections raises a column-binding error on SQL backends @@ -1289,6 +1290,19 @@ Total divergences tracked: **125** | Notes | Verified empirically (backlog item 87, 2026-08-13): Relation.cross_join() raises duckdb BinderException / sqlite OperationalError ("no such column") when the two input tables share a table name but originate from separate Ibis connections (exactly the shape produced by the test suite's BackendDataFrameFactory, which names every table "test_table"); ibis-polars and polars/narwhals are unaffected. Root cause not yet isolated to Ibis vs. mountainash's join visitor — likely a missing table-alias disambiguation when two DatabaseTable operations share a name across connections. Renamed from a colliding DivergenceFact id (was "IB-REL-09", whose yaml entry described a different, no-longer-reproducible "suffixes kwarg" TypeError) when backlog item 87's id-collision detector caught the reuse; the DivergenceFact and this entry now self-reference via upstream_ref. | | Last Verified | 2026-08-13 | +### IB-REL-13: ibis-sqlite: asof join raises UnsupportedOperationError + +| Field | Value | +|---|---| +| Project | ibis | +| Category | Relational Operations | +| Root Cause | upstream_feature_gap | +| Affected Backends | ibis-sqlite | +| Status | Needs Filing | +| Workaround | Strict xfail | +| Notes | Split from IB-REL-11 (2026-08-18) after the ibis-duckdb portion (a nondeterministic row-order bug, not a value divergence) was fixed in mountainash by ordering the join_asof result deterministically. This entry tracks the remaining genuine upstream gap: Ibis has no ASOF JOIN translation for the SQLite backend, so Table.asof_join() raises UnsupportedOperationError on ibis-sqlite. Use a polars or narwhals backend for asof joins on that data. | +| Last Verified | 2026-08-18 | + ## Aggregate Operations ### IB-AGG-01: No product aggregate function — no SQL standard equivalent diff --git a/docs/reference/expression-coverage.json b/docs/reference/expression-coverage.json index 3d9b2e75..c5fa843e 100644 --- a/docs/reference/expression-coverage.json +++ b/docs/reference/expression-coverage.json @@ -69926,32 +69926,31 @@ "since": "2026-08-06" }, { - "id": "IB-REL-11", - "kind": "semantics", + "id": "IB-REL-12", + "kind": "engine_leniency", "operation_keys": [], "backends": [ "ibis-duckdb", "ibis-sqlite" ], - "summary": "asof join is unreliable on ibis SQL backends: ibis-duckdb returns a wrong (diverging) result and ibis-sqlite raises UnsupportedOperationError", - "impact": "Relation.join_asof() diverges on ibis-duckdb and raises on ibis-sqlite; polars/narwhals compute it correctly", - "workaround": "Use a polars or narwhals backend for asof joins", - "upstream_ref": "IB-REL-11", + "summary": "cross_join between two same-named tables from different Ibis connections raises a column-binding error on ibis SQL backends (ibis-duckdb BinderException, ibis-sqlite OperationalError)", + "impact": "Relation.cross_join() raises on ibis-duckdb/ibis-sqlite; polars/narwhals and ibis-polars compute it", + "workaround": "Use a polars/narwhals backend or ibis-polars for cross joins", + "upstream_ref": "IB-REL-12", "since": "2026-08-06" }, { - "id": "IB-REL-12", + "id": "IB-REL-13", "kind": "engine_leniency", "operation_keys": [], "backends": [ - "ibis-duckdb", "ibis-sqlite" ], - "summary": "cross_join between two same-named tables from different Ibis connections raises a column-binding error on ibis SQL backends (ibis-duckdb BinderException, ibis-sqlite OperationalError)", - "impact": "Relation.cross_join() raises on ibis-duckdb/ibis-sqlite; polars/narwhals and ibis-polars compute it", - "workaround": "Use a polars/narwhals backend or ibis-polars for cross joins", - "upstream_ref": "IB-REL-12", - "since": "2026-08-06" + "summary": "ibis-sqlite: asof join raises UnsupportedOperationError — no ASOF JOIN translation", + "impact": "Relation.join_asof() raises on ibis-sqlite; polars/narwhals and ibis-duckdb compute it correctly", + "workaround": "Use a polars or narwhals backend for asof joins", + "upstream_ref": "IB-REL-13", + "since": "2026-08-18" }, { "id": "IB-STR-11", diff --git a/docs/reference/expression-coverage.md b/docs/reference/expression-coverage.md index 63114fa4..ef5428f7 100644 --- a/docs/reference/expression-coverage.md +++ b/docs/reference/expression-coverage.md @@ -538,8 +538,8 @@ Cells whose facts are all scoped (dialect / parameter / option / value-class) ha | IB-MATH-06 | engine_leniency | ibis-polars, ibis-duckdb | `SINH`, `COSH`, `TANH`, `ASINH`, `ACOSH`, `ATANH` | ibis-polars and ibis-duckdb lack hyperbolic math functions in the mountainash arithmetic system; these ops raise NotImplementedError (ibis-sqlite is a separate engine limitation, IB-MATH-02) | sinh/cosh/tanh/asinh/acosh/atanh raise on ibis-polars and ibis-duckdb; polars computes them | Use a polars backend for hyperbolic functions | — | 2026-08-06 | | IB-REL-06 | engine_leniency | ibis-duckdb | — | DuckDB rejects tables containing untyped all-NULL columns | Projections whose column is entirely null raise a type resolution error | cast the null column to an explicit type first | IB-REL-06 | 2026-07-05 | | IB-REL-10 | engine_leniency | ibis-sqlite | — | ibis-sqlite lacks array/pivot relational translations: drop_nans, unpivot/melt raise OperationNotDefinedError | Relation.drop_nans()/unpivot()/melt() raise on ibis-sqlite; other backends compute them | Use ibis-duckdb or a polars/narwhals backend for these relational ops | IB-REL-10 | 2026-08-06 | -| IB-REL-11 | semantics | ibis-duckdb, ibis-sqlite | — | asof join is unreliable on ibis SQL backends: ibis-duckdb returns a wrong (diverging) result and ibis-sqlite raises UnsupportedOperationError | Relation.join_asof() diverges on ibis-duckdb and raises on ibis-sqlite; polars/narwhals compute it correctly | Use a polars or narwhals backend for asof joins | IB-REL-11 | 2026-08-06 | | IB-REL-12 | engine_leniency | ibis-duckdb, ibis-sqlite | — | cross_join between two same-named tables from different Ibis connections raises a column-binding error on ibis SQL backends (ibis-duckdb BinderException, ibis-sqlite OperationalError) | Relation.cross_join() raises on ibis-duckdb/ibis-sqlite; polars/narwhals and ibis-polars compute it | Use a polars/narwhals backend or ibis-polars for cross joins | IB-REL-12 | 2026-08-06 | +| IB-REL-13 | engine_leniency | ibis-sqlite | — | ibis-sqlite: asof join raises UnsupportedOperationError — no ASOF JOIN translation | Relation.join_asof() raises on ibis-sqlite; polars/narwhals and ibis-duckdb compute it correctly | Use a polars or narwhals backend for asof joins | IB-REL-13 | 2026-08-18 | | IB-STR-11 | engine_leniency | ibis-polars | `CONTAINS`, `STARTS_WITH`, `ENDS_WITH` | ibis-polars rejects case-insensitive string matching (contains/starts_with/ends_with with case_sensitive=False) — UnsupportedArgumentError | case-insensitive contains/starts_with/ends_with raise on ibis-polars; other backends compute them | Use ibis-duckdb/ibis-sqlite or a polars/narwhals backend for case-insensitive matching | IB-STR-11 | 2026-08-06 | | IB-TYPE-02 | semantics | ibis-duckdb, ibis-sqlite | `IS_NAN`, `FILL_NAN` | SQL engines treat NaN as NULL; NaN == NaN yields NULL not False | is_nan/fill_nan/NaN comparisons diverge on SQL engines | Use is_null/fill_null on SQL backends | IB-TYPE-02 | 2026-07-05 | | IB-TYPE-04 | type_inference | ibis-duckdb, ibis-polars, ibis-sqlite | — | Ibis defers type resolution to its backend, unlike eager Polars | Type-sensitive operations and result comparisons can differ despite matching values | — | IB-TYPE-04 | 2026-07-05 | diff --git a/registry/upstream-issues.yaml b/registry/upstream-issues.yaml index c29da6c6..61e73c10 100644 --- a/registry/upstream-issues.yaml +++ b/registry/upstream-issues.yaml @@ -603,23 +603,47 @@ issues: - id: IB-REL-11 project: ibis category: relation-ops - summary: asof join is unreliable on Ibis SQL backends - root_cause: upstream_bug + summary: 'RESOLVED: ibis-duckdb asof join returned nondeterministic row order' + root_cause: mountainash_internal affected_backends: - ibis-duckdb + upstream_issue: null + upstream_issue_filed_by: null + status: resolved_in_mountainash + our_workaround: none + notes: 'Originally described as "asof join is unreliable on Ibis SQL backends" + covering both ibis-duckdb and ibis-sqlite. Root-caused (2026-08-18, PR #297 + CI investigation): Relation.join_asof() on the Ibis backend never applied an + explicit ORDER BY, and DuckDB gives no row-order guarantee for ASOF JOIN + output absent one — the query planner nondeterministically returned rows in + original or shuffled order run-to-run, flipping the strict-xfail test between + XFAIL and XPASS(strict) independent of any test-suite change. Not a value + divergence — the join values were always correct, only positional-assertion + row order flapped. Fixed by ordering the Ibis join_asof result by the join + key(s) (relsys_ib_ext_ma_util.py) to match polars/narwhals'' left-order + preservation. The distinct ibis-sqlite raise (SQLite has no ASOF JOIN + translation) was split off to IB-REL-13, which remains a genuine open gap. + Renamed from a colliding DivergenceFact id (was "IB-REL-08") 2026-08-13; + that history stands, this entry is now closed.' + last_verified: '2026-08-18' +- id: IB-REL-13 + project: ibis + category: relation-ops + summary: 'ibis-sqlite: asof join raises UnsupportedOperationError' + root_cause: upstream_feature_gap + affected_backends: - ibis-sqlite upstream_issue: null upstream_issue_filed_by: null - status: needs_investigation + status: needs_filing our_workaround: xfail_strict - notes: 'Relation.join_asof() returns a diverging (wrong) result on ibis-duckdb and - raises UnsupportedOperationError on ibis-sqlite; polars/narwhals compute it - correctly. Renamed from a colliding DivergenceFact id (was "IB-REL-08", which - already named an unrelated entry above — the backend rename-mapping-inverted - gap) when backlog item 87''s id-collision detector caught the reuse; the - DivergenceFact and this entry now self-reference via upstream_ref (backlog - item 87, 2026-08-13).' - last_verified: '2026-08-13' + notes: 'Split from IB-REL-11 (2026-08-18) after the ibis-duckdb portion (a + nondeterministic row-order bug, not a value divergence) was fixed in + mountainash by ordering the join_asof result deterministically. This entry + tracks the remaining genuine upstream gap: Ibis has no ASOF JOIN translation + for the SQLite backend, so Table.asof_join() raises UnsupportedOperationError + on ibis-sqlite. Use a polars or narwhals backend for asof joins on that data.' + last_verified: '2026-08-18' - id: IB-REL-12 project: ibis category: relation-ops diff --git a/src/mountainash/core/capabilities/divergences.py b/src/mountainash/core/capabilities/divergences.py index 005f1175..8aeabcbb 100644 --- a/src/mountainash/core/capabilities/divergences.py +++ b/src/mountainash/core/capabilities/divergences.py @@ -670,15 +670,15 @@ def _all() -> tuple[DivergenceFact, ...]: since="2026-08-06", ), DivergenceFact( - id="IB-REL-11", - kind=DivergenceKind.SEMANTICS, + id="IB-REL-13", + kind=DivergenceKind.ENGINE_LENIENCY, operation_keys=(), # relation op join_asof - backends=("ibis-duckdb", "ibis-sqlite"), - summary="asof join is unreliable on ibis SQL backends: ibis-duckdb returns a wrong (diverging) result and ibis-sqlite raises UnsupportedOperationError", - impact="Relation.join_asof() diverges on ibis-duckdb and raises on ibis-sqlite; polars/narwhals compute it correctly", + backends=("ibis-sqlite",), + summary="ibis-sqlite: asof join raises UnsupportedOperationError — no ASOF JOIN translation", + impact="Relation.join_asof() raises on ibis-sqlite; polars/narwhals and ibis-duckdb compute it correctly", workaround="Use a polars or narwhals backend for asof joins", - upstream_ref="IB-REL-11", - since="2026-08-06", + upstream_ref="IB-REL-13", + since="2026-08-18", ), DivergenceFact( id="IB-REL-12", 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 dc2b5ffb..7b8b85bc 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 @@ -197,4 +197,12 @@ def join_asof( kwargs["by"] = by if tolerance is not None: kwargs["tolerance"] = tolerance - return left.asof_join(right, **kwargs) + result = left.asof_join(right, **kwargs) + # Ibis's SQL backends (duckdb, sqlite) give no row-order guarantee for + # ASOF JOIN output absent an explicit ORDER BY. An asof join is defined + # over data pre-sorted by `on`, so restoring that order here is the + # correct, deterministic result — it matches polars/narwhals, which + # preserve left input order. Without this, results were flaky rather + # than wrong (see docs/known-divergences.md IB-REL-11, resolved). + order_cols = [*by, on] if by else [on] + return result.order_by(order_cols) diff --git a/tests/_spine_expectation_census.md b/tests/_spine_expectation_census.md index 9879c312..45932b46 100644 --- a/tests/_spine_expectation_census.md +++ b/tests/_spine_expectation_census.md @@ -146,7 +146,7 @@ Buckets: `migrated` (derivable from the spine today), `retained` (a LITERAL_ONLY | tests/relations/cross_backend/test_rel_extension_ops_results.py:39 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('NW-REL-01') — migrated | | tests/relations/cross_backend/test_rel_extension_ops_results.py:40 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('NW-REL-02') — migrated | | tests/relations/cross_backend/test_rel_extension_ops_results.py:41 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('MA-REL-01') — migrated | -| tests/relations/cross_backend/test_rel_join_results.py:42 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('IB-REL-11') — migrated | +| tests/relations/cross_backend/test_rel_join_results.py:42 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('IB-REL-13') — migrated | | tests/relations/cross_backend/test_terminal_scalar_aggregates.py:13 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('IB-AGG-05') — migrated | | tests/relations/cross_backend/test_terminal_scalar_aggregates.py:14 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('NW-AGG-03') — migrated | | tests/relations/test_relation_aliases.py:23 | static-marker | UNRESOLVED | UNRESOLVED | UNRESOLVED | None | spine-derived id-keyed divergence mark via xfail_divergence('IB-REL-10') — migrated | diff --git a/tests/relations/cross_backend/test_rel_join_results.py b/tests/relations/cross_backend/test_rel_join_results.py index 18629980..b2b670a9 100644 --- a/tests/relations/cross_backend/test_rel_join_results.py +++ b/tests/relations/cross_backend/test_rel_join_results.py @@ -39,7 +39,7 @@ def sorted_dicts(dicts: list[dict], by: str | list[str]) -> list[dict]: from fixtures.capability_gating import xfail_divergence _ASOF = [ - pytest.param(b, marks=xfail_divergence("IB-REL-11", backend=b)) for b in ALL_BACKENDS + pytest.param(b, marks=xfail_divergence("IB-REL-13", backend=b)) for b in ALL_BACKENDS ] @pytest.mark.cross_backend