Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 19 additions & 18 deletions core/wren-core/core/src/logical_plan/analyze/model_anlayze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,19 +447,16 @@ impl ModelAnalyzeRule {
) -> Result<Transformed<LogicalPlan>> {
match plan {
LogicalPlan::SubqueryAlias(SubqueryAlias { input, alias, .. }) => {
// Because the bottom-up transformation is used, the table_scan is already transformed
// to the ModelPlanNode before the SubqueryAlias. We should check the patten of Wren-generated model plan like:
// SubqueryAlias -> SubqueryAlias -> Extension -> ModelPlanNode
// to get the correct required columns
// `shortcut_aliased_table_scan` consumes `SubqueryAlias -> TableScan`
// over a model in `f_down`, so no model scan reaches the arms below.
// The `SubqueryAlias` arm rejects any `Extension` input and flattens
// everything else onto the outer alias; the `TableScan` arm sees only
// the scans the shortcut declined — tables outside the MDL, or inside
// it but not a model.
match Arc::unwrap_or_clone(Arc::clone(&input)) {
LogicalPlan::SubqueryAlias(subquery_alias) => self
.analyze_subquery_alias_model(
subquery_alias,
scope_manager,
current_scope_id,
alias,
cycle_stack,
),
LogicalPlan::SubqueryAlias(subquery_alias) => {
self.analyze_subquery_alias_model(subquery_alias, alias)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
LogicalPlan::TableScan(table_scan) => {
let model_plan = self
.analyze_table_scan(
Expand Down Expand Up @@ -788,22 +785,26 @@ impl ModelAnalyzeRule {
}
}

/// Rebuilds a `SubqueryAlias -> SubqueryAlias -> Extension -> ModelPlanNode` shape.
/// `shortcut_aliased_table_scan` (`f_down`) now intercepts every aliased `TableScan`
/// before that nested shape can be produced, so the branch below is defense-in-depth.
/// Handles the input of a `SubqueryAlias -> SubqueryAlias -> ...` shape. An
/// `Extension` input is rejected; any other input is flattened onto the outer
/// `alias`, dropping the inner one.
fn analyze_subquery_alias_model(
&self,
subquery_alias: SubqueryAlias,
_scope_manager: &mut ScopeManager,
_current_scope_id: ScopeId,
alias: TableReference,
_cycle_stack: &ModelStack,
) -> Result<Transformed<LogicalPlan>> {
let SubqueryAlias { input, .. } = subquery_alias;
if let LogicalPlan::Extension(Extension { node }) =
Arc::unwrap_or_clone(Arc::clone(&input))
{
if let Some(model_node) = node.as_any().downcast_ref::<ModelPlanNode>() {
// Unreachable: `shortcut_aliased_table_scan` consumes every
// `SubqueryAlias -> TableScan` over a model in `f_down`, and the only
// producer of a directly-nested `SubqueryAlias` 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` and this shape never
// forms. Revisit if view planning starts handing back optimized plans.
internal_err!(
"SubqueryAlias wrapping an already-analyzed ModelPlanNode ({}) \
reached the bottom-up rebuild path; shortcut_aliased_table_scan \
Expand Down
16 changes: 8 additions & 8 deletions core/wren-core/core/src/logical_plan/analyze/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1084,14 +1084,14 @@ impl ModelSourceNode {
}
// Prune columns the caller cannot access under column-level
// security instead of denying. This is an *implicit* all-columns
// expansion (a `count(*)`, or the throwaway inner plan built for
// a model scan referenced with a table alias, whose required
// columns are keyed by the alias so the model is wildcard-
// expanded). A protected column the query never explicitly
// selected must be dropped here — matching `SELECT *` semantics —
// not turned into a hard permission error. Explicit references
// still deny, because they arrive as named required fields (the
// non-wildcard branch / ModelPlanNodeBuilder::build), not here.
// expansion: the scan reached `ModelPlanNodeBuilder` with no
// required fields (a `count(*)`-shaped query), so a wildcard was
// substituted for them. A protected column the query never
// explicitly selected must be dropped here — matching `SELECT *`
// semantics — not turned into a hard permission error. Explicit
// references still deny, because they arrive as named required
// fields (the non-wildcard branch / ModelPlanNodeBuilder::build),
// not here.
let (is_valid, _) = validate_clac_rule(
model.name(),
&column,
Expand Down
Loading