- analyze_subquery_alias_model's ModelPlanNode branch is now unreachable
(shortcut_aliased_table_scan intercepts every SubqueryAlias -> TableScan
in f_down before the nested shape it rebuilds can be produced); turned
into an internal_err! defense-in-depth check instead of silent dead code
- reworded shortcut_aliased_table_scan's doc comment, which read as if the
old bottom-up path it replaces was still in service
- restated the rule doc's step-1/step-2 traversal-symmetry invariant in the
narrower form that actually holds now: a skipped subtree is safe only
because a bare TableScan can never carry a subquery
goldmedal verified that once the shortcut fires, the resulting node
(SubqueryAlias -> Extension(ModelPlanNode)) falls into
analyze_model_internal's catch-all arm and is only reconstructed via
SubqueryAlias::try_new, a no-op. Confirmed independently: full 149-test
suite passes with the field removed and f_up calling
analyze_model_internal unconditionally, cargo clippy -D warnings clean,
cargo fmt --check clean.
- Update the stale comment on test_clac_unreferenced_column_pruned_not_denied:
it described the throwaway-plan mechanism this PR deletes.
- Add snapshot assertions (assert_snapshot!) to the new test so it asserts the
no-behavior-change claim, not just the build-call count.
- Cover the two branches the new test missed: count(*) on an aliased model
(the visited_dataset fallback) and a join of two aliased models (shortcut_taken
across siblings).
- Drop the needless SubqueryAlias rebuild on non-matching paths in
shortcut_aliased_table_scan; match on &plan first and return the original
Transformed::no(plan) on every early exit.
- Comment the Jump/f_up invariant shortcut_taken depends on.
- Extract the required-columns/visited_dataset-fallback lookup duplicated
across shortcut_aliased_table_scan, analyze_table_scan and
analyze_subquery_alias_model into resolve_required_fields.
- Fix the counter's doc comment: the real dependency is #[tokio::test]'s
current-thread runtime, not "one thread per test".
Addresses goldmedal's review on PR #2612.
For a model scanned through a table alias (FROM customer AS e), the
bottom-up ModelAnalyzeRule walk visits the bare TableScan before its
SubqueryAlias parent. Required columns are keyed by the alias, so this
first pass finds none and builds a ModelPlanNode via the wildcard-
expansion path; analyze_subquery_alias_model then discards it and
rebuilds the node correctly once the alias is known. The first build
runs the full RLAC/CLAC parsing for nothing.
PR #2449 already fixed the correctness bug this caused (the throwaway
build was denying access to columns under column-level security instead
of pruning them); the wasted build itself remained.
This adds a pre-order TreeNodeRewriter pass that recognizes
SubqueryAlias -> TableScan for a model directly and builds the
ModelPlanNode once, with the alias's real required columns, skipping
the discarded intermediate build. Everything else keeps using the
original bottom-up analyze_model_internal pass.
Fixes#2451
Amir Fathi