fix(_xorq): don't let optional plain-ibis interop deactivate the xorq branch - #291
Merged
Conversation
… 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
marked this pull request as ready for review
July 30, 2026 19:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
from xorq.common.utils.ibis_utils import from_ibis, map_ibissat inside the all-or-nothingHAS_XORQimport block in_xorq.py. But xorq'sibis_utilsimports 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_XORQreadFalseeven though xorq was fully importableto_taggeddied atserialization/__init__.py:114withAttributeError: 'Table' object has no attribute 'replace'This is a dependency-activation bug, not an API mismatch — xorq's own
node_utils.replace_nodesacceptsExpr | Nodeon both xorq 0.3.34 and 0.3.37, where the failure was observed.Changes
from_ibis/map_ibisinto their own guard, afterHAS_XORQ = Trueis set. When the plain-ibis stack is absent, only those two interop symbols degrade to raising stubs —_MapIbisXorqStubis registry-preserving so_patch_xorq_sortkey_compat()can still register handlers, matching the existing no-xorq_MapIbisStubcontract.replace_nodesto normalizeExpr → opviato_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
HAS_XORQTrue, xorq's nativereplace_nodesused,to_taggedworks over adeferred_read_csvsource.main.Note
This branch was originally stacked on
fix/serialization-rce-and-roundtripand has been rebased directly ontomain— it is independent of that work (disjoint files) and can merge on its own.🤖 Generated with Claude Code