Repository navigation
fix(engine): multi-clause rules sharing a derived relation (#91) - #123
Merged
Merged
Conversation
RuleCatalog stored definitions in a HashMap, so all_rules() emitted rules in hash order. The topological sort keeps that order for rules in a cycle, so mutually recursive rules ran in a different order per process and produced different results. Store definitions in a BTreeMap so iteration (and catalog.json) is ordered by rule name.
Root cause: JoinPlanner::plan_joins treated a multi-clause rule's Union as one conjunctive join block. extract_scans descended into every Union branch, so scans from different clauses landed in one join graph, and variables with the same name in different clauses (E1/E2 in the bridge and solo clauses) became join edges. When that graph was connected, the planner rebuilt a single join tree across clauses and preserve_top_operations returned it in place of the Union, dropping each branch's Map/Compute/Filter. Result: constant filters of one clause applied to the other (silently empty), or the raw cross-clause join leaked out (4 columns for a 3-column head). Clauses with disjoint variable names left the graph disconnected, which is why only some shapes failed. Planner is on by default and the server ignores [optimization] in config, so disabling it there had no effect. Fix: plan each Union branch as its own join block, and treat a Union nested under a join as an opaque leaf. extract_scans now returns the leaf IR nodes directly. Snapshots that recorded the bug are updated: the two-hop session rule (malformed 3-column output), the recursive debug plan (union merged into a join), and the mutual recursion cases (empty). Mutual recursion now returns base cases but still does not iterate across relations; only self-recursive heads run to fixpoint. That is a separate defect.
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.
Fixes #91.
Root cause:
JoinPlanner::plan_joins(src/join_planning/mod.rs) treated a multi-clause rule'sUnionas one join block. Scans from different clauses landed in one join graph, and same-named variables across clauses became join edges. When that graph was connected, the planner replaced theUnionwith one cross-clause join, dropping each clause's projections and filters: empty results, or a raw cross join with extra columns. Disjoint variable names left the graph disconnected, which is why only some shapes failed.Fix
BTreeMapso persistent rule order is deterministic (mutual-recursion results were hash-order dependent once the fix landed).Tests:
tests/multi_clause_rule_tests.rs(8, viaHandler: both issue repros, retroactive empty-on-new-clause, session rules, consistency-coreconflictas one 4-clause rule, negation, bound recursion, aggregate; 6 fail on main). 2 planner unit tests. New snapshot18_advanced_patterns/98_multi_clause_shared_derived; 5 snapshots that recorded the bug as expected output updated.cargo test: 3,286 passed; snapshots: 1,145 passed, stable over 5 runs.Found, not fixed here
[optimization]config (IQLEngine::new()always), so the flags in the issue comment did nothing.Var < Varis always false:ColumnsLt(src/code_generator/mod.rs:1821) only compares numbers.src/lib.rs:1252).consistency-core's one-clause-per-relation workaround can now be collapsed.