Skip to content

fix(wren): case-insensitive CTE name shadowing in CTERewriter#1489

Open
goldmedal wants to merge 1 commit intoCanner:mainfrom
goldmedal:claude/nervous-lichterman
Open

fix(wren): case-insensitive CTE name shadowing in CTERewriter#1489
goldmedal wants to merge 1 commit intoCanner:mainfrom
goldmedal:claude/nervous-lichterman

Conversation

@goldmedal
Copy link
Copy Markdown
Contributor

@goldmedal goldmedal commented Mar 30, 2026

Summary

  • Fix duplicate CTE generation when a user-defined CTE name differs only in case from an MDL model name (e.g. WITH Orders AS (...) vs model orders)
  • _collect_user_cte_names() now lowercases names to match sqlglot's normalize_identifiers behavior
  • Add test_user_cte_shadows_model_case_insensitive reproducing the exact scenario from feat(wren): CTE-based SQL planning with per-model expansion #1479 (comment)

Addresses #1479 (comment)

Test plan

  • New test test_user_cte_shadows_model_case_insensitive passes
  • All 17 existing CTE rewriter unit tests pass
  • CI passes

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed case-insensitive handling of CTE names to prevent duplicate generation when user-defined CTEs differ in case from model names.

_collect_user_cte_names() collected raw CTE names (e.g. "Orders")
while _collect_model_columns() compared against normalized/lowercased
table names ("orders"). The case-sensitive check missed the shadow,
generating a duplicate CTE. Lowercase names on collection to match
sqlglot's normalize_identifiers behavior.

Closes Canner#1479 (comment)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cf3f3d86-7607-477a-a679-1612cf4a0beb

📥 Commits

Reviewing files that changed from the base of the PR and between 6a78c12 and fdefa12.

📒 Files selected for processing (2)
  • wren/src/wren/mdl/cte_rewriter.py
  • wren/tests/unit/test_cte_rewriter.py

📝 Walkthrough

Walkthrough

A case normalization fix for CTE (Common Table Expression) name comparison in the MDL rewriter. The _collect_user_cte_names function now converts collected CTE names to lowercase before comparison with model table names, preventing duplicate CTE generation when names differ only in case. A corresponding test case validates this behavior.

Changes

Cohort / File(s) Summary
CTE Name Normalization
wren/src/wren/mdl/cte_rewriter.py
Modified _collect_user_cte_names to normalize user-defined CTE names to lowercase via .lower() before insertion into the returned set, ensuring case-insensitive comparison against model table names.
Test Coverage
wren/tests/unit/test_cte_rewriter.py
Added unit test test_user_cte_shadows_model_case_insensitive that verifies duplicate CTE generation is prevented when a user CTE name differs only in case from a model name (e.g., Orders vs orders).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

core, python

Suggested reviewers

  • douenergy

Poem

🐰 A rabbit hops through CTE trees,
Where Orders and orders once disagreed,
With .lower() cast, the shadows align,
No duplicate CTEs cause design,
Case-insensitive logic now pristine! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing case-insensitive CTE name shadowing in the CTERewriter, which directly matches the core modification to normalize CTE names to lowercase and the test case validating this behavior.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@goldmedal goldmedal requested a review from douenergy March 30, 2026 06:17
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.

1 participant