mirror of
https://github.com/Canner/WrenAI.git
synced 2026-08-29 08:18:10 +08:00
refactor(core): correct the aliased-scan comments the shortcut invalidated (#2648)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
}
|
||||
LogicalPlan::TableScan(table_scan) => {
|
||||
let model_plan = self
|
||||
.analyze_table_scan(
|
||||
@@ -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 \
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user