fix(plan): add explainer visit method for PhysicalLayerSchemaCreationStage - #5925
nkwork9999 wants to merge 2 commits into
Conversation
53cd09a to
6cffcaf
Compare
6cffcaf to
40e26aa
Compare
…Stage `RichExplainerConsole` had no `visit_physical_layer_schema_creation_stage`, so explaining a plan that includes `PhysicalLayerSchemaCreationStage` logged `ERROR - Unexpected stage: PhysicalLayerSchemaCreationStage` and silently omitted the stage from the explained plan. Add the missing visit method. It mirrors the schema derivation in `SnapshotEvaluator.create_physical_schemas` (non-symbolic models only, table name resolved through the deployability index) and lists the distinct physical schemas that will be created. `test_plan_explain` now asserts that no stage is reported as unexpected, which covers this stage and guards against the same omission for future ones. Fixes SQLMesh#5619 Signed-off-by: nkwork9999 <143652584+nkwork9999@users.noreply.github.com>
40e26aa to
5b8823c
Compare
|
@nkwork9999 Thanks for this pr! No blocker, but do you think it's worth asserting the explained output includes the schema-creation node (e.g. “Create physical schemas…” / |
|
@nkwork9999 Just wanted to follow up on this! |
|
@nkwork9999 Bump |
Signed-off-by: nkwork9999 <143652584+nkwork9999@users.noreply.github.com>
|
Thanks for following up, @StuffbyYuki. I have pushed the requested test update in 2841c99. I also verified that replacing the schema-creation visitor with an empty renderer makes the new output assertion fail, even without an unexpected-stage error. The related plan, plan-stage, context, and plan-options suites pass: 214 tests. Ruff lint/format and the migration-number check pass. The full |
Fixes #5619
Problem
RichExplainerConsolehas novisit_physical_layer_schema_creation_stage, so when a plan containsPhysicalLayerSchemaCreationStage,explain()falls into thehasattrguard atexplainer.py:134:The stage is then silently dropped from the explained plan, even though
PlanEvaluatorhandles it inevaluator.py:209.Change
Add the missing visit method. It mirrors the schema derivation in
SnapshotEvaluator.create_physical_schemas/_create_schemas— non-symbolic models only, table name resolved through the stage's deployability index, deduplicated by(db, catalog)— and lists the distinct physical schemas that will be created. ReturnsNonewhen there is nothing to create, consistent with the otherOptional[Tree]visit methods.Placed directly before
visit_physical_layer_update_stageto match the stage ordering produced instages.py.Output on
examples/sushi:Tests
test_plan_explainpreviously only asserted that explaining does not raise, which is why this went unnoticed. It now asserts that no stage is reported as unexpected, which covers this stage and guards against the same omission for stages added later.Verified the test fails on
mainwith the fix reverted:tests/core/test_plan.py,tests/core/test_plan_stages.py,tests/core/test_context.pyandtests/core/integration/test_plan_options.pypass (207 tests), as doruff format,ruff checkandmypyon the changed files.