refactor(core): correct the aliased-scan comments the shortcut invalidated

[goldmedal]

Summary

Follow-up to #2612. That PR replaced the bottom-up rewrite of SubqueryAlias -> TableScan
with a pre-order shortcut, but three comments still describe the path it removed. All three
sit on code a reader consults to understand behavior they cannot infer from the code alone,
so they are worth correcting rather than leaving to drift.

• ModelSourceNode::new (plan.rs) documented the wildcard-prune branch as reached by
  "a count(*), or the throwaway inner plan built for a model scan referenced with a table
  alias". refactor(core): avoid throwaway ModelPlanNode build for aliased model scans #2612 deleted that second trigger. The wildcard is substituted in exactly one place
  (ModelPlanNodeBuilder::build, when required_fields.is_empty()), so only the
  count(*)-shaped case reaches it now. This comment carries the column-level-security
  prune-vs-deny rationale from fix(core): prune inaccessible CLS columns in wildcard expansion instead of denying #2449, which is why a stale trigger list there matters more
  than a normal stale comment.
• analyze_subquery_alias_model's doc still opened with "Rebuilds a
  SubqueryAlias -> SubqueryAlias -> Extension -> ModelPlanNode shape". It rejects that
  shape now.
• analyze_model_internal's SubqueryAlias arm still explained itself as "because the
  bottom-up transformation is used, the table_scan is already transformed to the
  ModelPlanNode before the SubqueryAlias" — which is what the shortcut stopped doing.

It also writes down the invariant the internal_err! rests on. The existing message implies
the branch is unreachable because the shortcut intercepts every shape; that is not quite the
reason. The shortcut only declines to fire on a directly nested SubqueryAlias, and the
one thing that produces that shape here is ExpandWrenViewRule, which plans view bodies with
SessionState::create_logical_plan — unoptimized. A view body therefore always has a
Projection root, so the inner node is never a bare TableScan. That is a property of how
views are planned today, not something this rule owns, so it is now stated with a pointer to
revisit if view planning changes.

analyze_subquery_alias_model's three _-prefixed parameters, dead since #2612, go with it.

What failure does this repair?

None. Comments plus one private signature; no plan, SQL, or public API changes.

How is it tested?

No new tests — there is no behavior to assert. The parameter removal is compiler-checked and
the rest is comments.

• RUST_MIN_STACK=8388608 cargo test --lib --tests --bins — 149 passed
• cargo test -p wren-sqllogictest — tpch.slt, model.slt, view.slt, type.slt pass
• cargo clippy --all-targets --all-features -- -D warnings — clean
• cargo fmt --check — clean; working tree clean after the test run

Separately verified while reviewing #2612: 20 query shapes (view + alias, view count(*),
CTE with an outer alias, nested subquery alias, self-join, view-join-model, IN / EXISTS /
correlated-scalar subqueries, UNION ALL, column aliases) produce byte-identical SQL against
that PR's merge-base, and none of them reaches the internal_err! branch.

Not checked: whether any shape outside those 20 can form a directly-nested
SubqueryAlias -> SubqueryAlias -> TableScan. The claim above is structural, not exhaustive.

Summary by CodeRabbit

• Documentation

  • Clarified wildcard column expansion and inaccessible-column handling in model sources.
  • Updated guidance for nested subquery alias analysis.

• Refactor

  • Simplified nested subquery-alias model handling without changing user-visible behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5c8bc541-bca6-4320-93df-7cd5fc0e1e4c

📥 Commits

Reviewing files that changed from the base of the PR and between 17d21aa and 2f45adc.

📒 Files selected for processing (1)

• core/wren-core/core/src/logical_plan/analyze/model_anlayze.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• core/wren-core/core/src/logical_plan/analyze/model_anlayze.rs

---

Walkthrough

The change simplifies nested model alias analysis after aliased table scans use the pre-order shortcut. It also clarifies wildcard expansion and inaccessible-column handling in ModelSourceNode::new documentation.

Changes

Model analysis updates

Layer / File(s)	Summary
Simplify nested model alias analysis core/wren-core/core/src/logical_plan/analyze/model_anlayze.rs	Aliased table scans use the pre-order shortcut. analyze_subquery_alias_model now accepts only the subquery alias and alias, validates model extensions, and preserves other inputs.
Clarify wildcard selection rules core/wren-core/core/src/logical_plan/analyze/plan.rs	Comments explain wildcard expansion, pruning of inaccessible columns for implicit selection, and denial of explicit inaccessible references.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• refactor(core): avoid throwaway ModelPlanNode build for aliased model scans #2451 — Concerns aliased model-scan handling and avoids unnecessary model-plan construction.

Possibly related PRs

• Canner/WrenAI#2335 — Earlier changes modified nested model expansion and cycle handling.
• Canner/WrenAI#2449 — Related wildcard expansion behavior for inaccessible columns.
• Canner/WrenAI#2612 — Introduced the aliased model-scan pre-order shortcut.

Poem

A rabbit checks the model trail,
Nested aliases now prevail.
Wildcards expand with columns clear,
Explicit blocked names stay near.
Shorter paths make logic bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the core change: correcting comments invalidated by the aliased-scan shortcut.
Description check	✅ Passed	The description covers the change, non-behavioral scope, rationale, and testing, but omits the Duplicate check section.
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.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/aliased-scan-shortcut-followups

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@core/wren-core/core/src/logical_plan/analyze/model_anlayze.rs`:
- Around line 450-458: Update the branch comment above the LogicalPlan match in
the model-analysis fallback to reflect the actual conditions:
analyze_subquery_alias_model rejects any Extension input, while its
non-ModelPlanNode path calls internal_err!, and the TableScan arm can still
receive MDL scans when get_model is absent and shortcut_aliased_table_scan
returns unchanged.
- Around line 787-805: Update the documentation for analyze_subquery_alias_model
to state that the non-Extension fallback flattens nested aliases by discarding
the inner alias and rebuilding the plan with the outer alias parameter. Keep the
existing Extension rejection behavior and implementation unchanged.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ea47c17-ee7e-4a6e-ad37-b8f1bb13eb1b

📥 Commits

Reviewing files that changed from the base of the PR and between 1742175 and 17d21aa.

📒 Files selected for processing (2)

• core/wren-core/core/src/logical_plan/analyze/model_anlayze.rs
• core/wren-core/core/src/logical_plan/analyze/plan.rs