Skip to content

fix(wren-core): preserve composite-key predicates in relationship joins#2323

Open
goldmedal wants to merge 2 commits into
mainfrom
worktree-plan-0525-115336
Open

fix(wren-core): preserve composite-key predicates in relationship joins#2323
goldmedal wants to merge 2 commits into
mainfrom
worktree-plan-0525-115336

Conversation

@goldmedal
Copy link
Copy Markdown
Collaborator

@goldmedal goldmedal commented May 25, 2026

Summary

  • RelationChain::plan extracted all identifiers from a relationship condition and then used only the first two — so a.x = b.x AND a.y = b.y lost the second equality, breaking composite-key joins. The TPCH partsupp fixture in this repo already carries a TODO noting the gap (// ps_partkey and ps_suppkey should be composite primary key).
  • Add collect_join_keys (new helper in mdl/utils.rs) that parses the condition expression and returns equality pairs in AST order, rejecting non-equality / disjunction / non-column shapes with a clear error.
  • relation_chain.rs now rebases each column to the right alias (same logic as before) and ANDs the per-pair equalities into the final join predicate.
  • This is the Phase 1 prerequisite for declarative composite primary keys (separate follow-up); on its own it already unlocks composite-join condition strings, which were previously expressible in MDL but silently truncated.

Test plan

  • cargo fmt --all
  • cargo clippy --all-targets --all-features -- -D warnings
  • RUST_MIN_STACK=8388608 cargo test --lib --tests --bins --release — 133 passed (8 new helper tests + 1 new e2e composite-key test)
  • sqllogictest suites (view.slt, model.slt, tpch/tpch.slt, type.slt) all pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Support composite relationship keys: joins can be defined by multiple column equalities.
  • Bug Fixes

    • Preserve multi-column join predicates during SQL transformation so all key equalities are included.
  • Tests

    • Added unit and integration tests validating single- and multi-key join conditions, error cases, and snapshot coverage.

Review Change Stack

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file python Pull requests that update Python code rust Pull requests that update rust code core labels May 25, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b5ecb6ba-b95e-4308-ab50-935890f38e9e

📥 Commits

Reviewing files that changed from the base of the PR and between 1cba5ef and 05febe4.

📒 Files selected for processing (1)
  • core/wren-core/core/src/mdl/mod.rs

Walkthrough

Parses SQL join condition strings into ordered column-equality pairs, updates relation chain logic to rebase and AND-conjoin all equality predicates for composite-key relationships, and adds an end-to-end test validating composite join predicates appear in generated SQL.

Changes

Composite Key Join Condition Support

Layer / File(s) Summary
Join key extraction utilities
core/wren-core/core/src/mdl/utils.rs
New public function collect_join_keys parses SQL join condition strings into DataFusion expressions, recursively extracts ordered column pairs from AND-conjoined equality predicates, and rejects non-equality operators, disjunctions, and non-column operands. Unit tests validate single-key, composite/multi-key, parenthesized, and quoted-identifier extraction plus error handling for > operators, OR disjunctions, and literal operands.
Relation chain join predicate construction
core/wren-core/core/src/logical_plan/analyze/relation_chain.rs
Updated imports to use collect_join_keys and DataFusionError. Join predicate logic now parses relationship conditions into key pairs, rebases each pair to the correct table alias, and combines all equalities with AND conjunctions, replacing the previous single-equality assumption.
Composite-key relationship integration test
core/wren-core/core/src/mdl/mod.rs
New async test test_composite_key_relationship builds a manifest with a composite-key relationship between partsupp and part, runs the SQL transformation for a calculated field, and snapshot-tests that both equality predicates appear in the generated ON clause conjoined with AND.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I nibble through predicates bright,
Equalities pair up, snug and tight,
AND by AND the join lines sing,
Composite keys make my heart spring! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(wren-core): preserve composite-key predicates in relationship joins' directly and clearly describes the main change: fixing how relationship joins handle composite-key predicates by preserving all equality conditions instead of truncating them.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-plan-0525-115336

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Relationship `condition` strings such as `a.x = b.x AND a.y = b.y` parsed
fine but were collapsed by `RelationChain::plan` into a single equality —
the planner extracted all identifiers and then used only the first two,
silently dropping any further conjuncts. The TPCH partsupp fixture
already carried a `// composite primary key` TODO noting this gap.

Add `collect_join_keys`, which parses the condition expression and
returns equality pairs in AST order, rejecting non-equality / disjunction
shapes with a clear error. The relation_chain planner now rebases each
column to the right alias as before and ANDs the per-pair equalities
into the final join predicate.

Covered by new unit tests for the parser and an end-to-end test asserting
both predicates appear in the generated SQL for a composite-key
relationship.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@goldmedal goldmedal force-pushed the worktree-plan-0525-115336 branch from 6d3362b to 1cba5ef Compare May 25, 2026 06:02
Replace the substring assertions in test_composite_key_relationship with
an inline assert_snapshot! over the full transformed SQL, matching the
style of other relationship/calculation tests in this module.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@goldmedal goldmedal requested a review from douenergy May 25, 2026 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core dependencies Pull requests that update a dependency file python Pull requests that update Python code rust Pull requests that update rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant