Skip to content

refactor: map structured plan appendices from spannerplan - #1017

Merged
apstndb merged 4 commits into
mainfrom
refactor/structured-plan-appendices
Sep 17, 2026
Merged

apstndb merged 4 commits into
mainfrom
refactor/structured-plan-appendices

Conversation

@apstndb

@apstndb apstndb commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Replace the CLI's duplicated EXPLAIN appendix formatter with reference.BuildAppendices from released spannerplan v0.3.1. The CLI maps structured sections into ResultAppendix and the legacy predicate slice, and propagates builder errors.

Existing section overrides, explicit empty/NONE selection, predicate markers, raw scalar descriptions, ordering, whitespace and nil results are preserved. The change does not enable scalar-variable resolution or alter default metadata display.

Tests retain CLI option selection, representative literal output, real-plan integration and the specific invalid-section error cause. Duplicate classification/formatting cases remain covered by the library.

Validation: make check with Go 1.26.8 and golangci-lint 2.13.2, focused appendix tests, independent current-head source review, and all required CI checks including the full race suite. A local race run failed during an existing 30 ms transaction-timeout test's setup; its isolated rerun and the final-head CI race suite passed.

Physical line changes against 1b8003f: application production -120, tests -2, fixtures unchanged. Including the library's +56 production / +88 test-example lines gives combined production -64, tests +86, total +22. This consolidates duplicated implementation; it is not a net reduction in total maintained lines.

Adopt github.com/apstndb/spannerplan@195dc13 (PR 65) via a temporary
pseudo-version and replace the local appendix formatter with a small
mapping to ResultAppendix. Propagate BuildAppendices errors through
processPlan, buildQueryPlanAppendix, and buildExplainAnalyzeResult.
Keep CLI section resolution, predicate markers, and query-mode output
unchanged. Do not enable scalar-variable resolution.
Preserve the historical Result.Appendices nil contract for EXPLAIN
output that has no selected appendix lines.
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown

Code Metrics Report

📊 View detailed coverage report (available for 7 days)

main (7c2ac86) #1017 (ab5f872) +/-
Coverage 87.4% 87.4% -0.1%
Code to Test Ratio 1:2.5 1:2.5 +0.0
Test Execution Time 2m38s 3m10s +32s
Details
  |                     | main (7c2ac86) | #1017 (ab5f872) |  +/-  |
  |---------------------|----------------|-----------------|-------|
- | Coverage            |          87.4% |           87.4% | -0.1% |
  |   Files             |            132 |             132 |     0 |
  |   Lines             |          13287 |           13235 |   -52 |
- |   Covered           |          11625 |           11579 |   -46 |
+ | Code to Test Ratio  |          1:2.5 |           1:2.5 |  +0.0 |
  |   Code              |          28894 |           28783 |  -111 |
- |   Test              |          72564 |           72561 |    -3 |
- | Test Execution Time |          2m38s |           3m10s |  +32s |

Code coverage of files in pull request scope (88.9% → 89.0%, patch 100.0%)

Files Coverage +/- Patch Coverage Status
internal/mycli/execute_partitioned.go 91.5% +0.8% - affected
internal/mycli/explain_print_sections.go 91.3% -0.7% 100.0% modified
internal/mycli/statements_explain_describe.go 87.9% +0.3% 100.0% modified

Reported by octocov

Replace the temporary pseudo-version with the released
github.com/apstndb/spannerplan@v0.3.1 (69dd04e). Assert literal
predicate and ordering appendix lines, and require the specific
full-section validation error instead of any non-nil error.
@apstndb

apstndb commented Sep 15, 2026

Copy link
Copy Markdown
Owner Author

Reviewed f1749b6: no actionable findings. The final release pin and stronger literal/error-cause assertions are covered by current-head review. Independent make check and all required CI checks passed; no unresolved review threads remain. The existing 30 ms setup-sensitive race-test failure is recorded in the PR validation note rather than treated as a passing local race run.

@apstndb
apstndb marked this pull request as ready for review September 15, 2026 19:10
Resolve explain_print_sections.go by keeping BuildAppendices mapping
and dropping the local appendix helpers reintroduced on main.
Retain released spannerplan v0.3.1.
@apstndb

apstndb commented Sep 17, 2026

Copy link
Copy Markdown
Owner Author

Reviewed 0096fbe3ebe25810bd700d9a7efad364c2baf552 against main 7fadf3cc61ee509747ae7a5e8f4b5602b631c2dc: no actionable findings in the inspected scope.

The integration conflict correctly retains the removal of local appendix helpers now supplied by released spannerplan v0.3.1. Explicit empty print sections, raw scalar descriptions, ordered appendix mapping, legacy predicates, and error propagation remain covered. The other integration changes match the automatic main merge. Independent make check passed on this exact HEAD; all nine applicable hosted checks passed.

An earlier local run exposed an existing telemetry shutdown diagnostic test failure; that failed log is retained, and the focused rerun, complete rerun, independent check, and hosted checks passed. The telemetry test concern is tracked separately from this appendix change.

@apstndb
apstndb merged commit 256ad16 into main Sep 17, 2026
10 checks passed
@apstndb
apstndb deleted the refactor/structured-plan-appendices branch September 17, 2026 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant