Skip to content

fix(_xorq): don't let optional plain-ibis interop deactivate the xorq branch - #291

Merged
hachej merged 1 commit into
mainfrom
fix/xorq-branch-activation
Jul 30, 2026
Merged

fix(_xorq): don't let optional plain-ibis interop deactivate the xorq branch#291
hachej merged 1 commit into
mainfrom
fix/xorq-branch-activation

Conversation

@hussainsultan

Copy link
Copy Markdown
Collaborator

Problem

from xorq.common.utils.ibis_utils import from_ibis, map_ibis sat inside the all-or-nothing HAS_XORQ import block in _xorq.py. But xorq's ibis_utils imports the plain ibis-framework stack (ibis, packaging, pyarrow_hotfix) at module scope.

In a minimal environment (xorq + BSL only, no plain ibis), that import raises ImportError — which took the entire xorq branch down with it. The result:

  • HAS_XORQ read False even though xorq was fully importable
  • the plain-ibis fallback shims ran against xorq objects
  • to_tagged died at serialization/__init__.py:114 with AttributeError: 'Table' object has no attribute 'replace'

This is a dependency-activation bug, not an API mismatch — xorq's own node_utils.replace_nodes accepts Expr | Node on both xorq 0.3.34 and 0.3.37, where the failure was observed.

Changes

  1. Move from_ibis/map_ibis into their own guard, after HAS_XORQ = True is set. When the plain-ibis stack is absent, only those two interop symbols degrade to raising stubs — _MapIbisXorqStub is registry-preserving so _patch_xorq_sortkey_compat() can still register handlers, matching the existing no-xorq _MapIbisStub contract.
  2. Harden the no-xorq fallback replace_nodes to normalize Expr → op via to_node, matching xorq's native signature, so it can't crash on an expression even if a future un-gated caller hands it one.

Net: src/boring_semantic_layer/_xorq.py, +46/-2. No other files touched.

Validation

  • xorq==0.3.34 + BSL with no plain ibis installed → HAS_XORQ True, xorq's native replace_nodes used, to_tagged works over a deferred_read_csv source.
  • Plain-ibis-only env unchanged; the fallback now accepts both op and expr.
  • 159 xorq / serialization / tagged tests pass on this branch standing on main.

Note

This branch was originally stacked on fix/serialization-rce-and-roundtrip and has been rebased directly onto main — it is independent of that work (disjoint files) and can merge on its own.

🤖 Generated with Claude Code

… branch

`from xorq.common.utils.ibis_utils import from_ibis, map_ibis` sat inside
the all-or-nothing HAS_XORQ import block, but xorq's ibis_utils imports
the plain ibis-framework stack (ibis, packaging, pyarrow_hotfix) at
module scope. In a minimal env (xorq + BSL only) that import raises, the
ENTIRE xorq branch silently deactivated — HAS_XORQ read False with xorq
fully importable — and the plain-ibis fallback shims ran against xorq
objects: to_tagged died at serialization/__init__.py:114 with
"AttributeError: 'Table' object has no attribute 'replace'" (observed
against xorq 0.3.34 and 0.3.37 alike; this is dependency activation, not
an API mismatch — xorq's own node_utils.replace_nodes accepts Expr|Node
on both versions).

Two changes:
- Import from_ibis/map_ibis in their own guard after HAS_XORQ is set;
  when the plain-ibis stack is absent only those two interop symbols
  degrade to raising stubs (registry-preserving, same contract as the
  no-xorq _MapIbisStub).
- Harden the no-xorq fallback replace_nodes to normalize Expr → op via
  to_node, matching xorq's native signature, so it can never crash on an
  expression even if a future un-gated caller hands it one.

Validated: xorq==0.3.34 + BSL, NO plain ibis → HAS_XORQ True, xorq's
replace_nodes used, to_tagged works over a deferred_read_csv source;
plain-ibis-only env unchanged (fallback accepts op AND expr); 170
xorq/serialization/tagged tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hussainsultan
hussainsultan marked this pull request as ready for review July 30, 2026 19:57
@hachej
hachej merged commit 9d391ab into main Jul 30, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants