Skip to content

Add OR (disjunction) to egglog rule bodies - #1

Draft
oflatt-claude wants to merge 1 commit into
saulshanabrook:mainfrom
oflatt-claude:add-or-disjunction
Draft

Add OR (disjunction) to egglog rule bodies#1
oflatt-claude wants to merge 1 commit into
saulshanabrook:mainfrom
oflatt-claude:add-or-disjunction

Conversation

@oflatt-claude

Copy link
Copy Markdown

Draft. Adds disjunction (OR) to egglog rule bodies. All changes are confined to the egglog/ subtree (subtree base 7e926c6e).

A rule query may contain (OR (branch) (branch) ...), where each branch is a conjunction of facts; the rule matches when any branch matches. Only variables common to every branch — or bound by the surrounding conjunction — may be used outside the OR. Correlated branches (referencing the surrounding query) are supported and compile to a fused, deduplicating, seminaive/delta-driven union node in the core-relations free-join engine (not rule-splitting, not an egglog→egglog rewrite).

  • Frontend: GenericFact::Or + parser; typecheck resolves branches (branch-local renaming, common-variable interface rule).
  • Backend: JoinStage::Union in core-relations; union-aware seminaive expansion in egglog-bridge; dedup on the output tuple.

Verified on the underlying egglog tree: tests/disjunction.rs 17/17 (incl. dedup-once and delta-driven proofs), full .egg harness 747/747, make nits clean. egglog-experimental is unaffected (it depends on a git-pinned egglog rev, not this subtree).

🤖 Generated with Claude Code

Applies the disjunction feature to the egglog subtree: a rule query may
contain (OR (branch) (branch) ...), where each branch is a conjunction of
facts; the rule matches when any branch matches, and only variables common to
every branch (or bound by the surrounding conjunction) may be used outside the
OR. Correlated branches are supported and compile to a fused, deduplicating,
seminaive/delta-driven union node in the core-relations free-join engine.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
}
}
JoinStage::Union { branches } => {
// Fused disjunction. Enumerate every branch independently — each

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extra materialization step is going to hurt performance I think.

maybe this'd be a good fallback, but we probably want to recognize the special case where all of the branches come from the same function, and we can build a special index to evaluate all branches at once.

For the general case, it may be better to simply "duplicate" the plan implicitly (i.e. just have each branch terminate in the action, not in a materialization).

@ezrosent ezrosent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This + other comment are more high-level feedback. It'd be good to ask claude about benchmarking this against:

  • a fast path with a custom index
  • a slow path that just desugars

Rather than doing this materialization path.

/// the one deduplicating materialization and the shared action — so a row
/// matched via several branches is still processed once.
///
/// Requires a cached union plan whose sole block-0 stage is a `Union` (as

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that core-relations should know about semi-naive... why can't we add semi-naive constraints to each disjunct in the OR in egglog-bridge?

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.

3 participants