fix: retain resolved table schemas in advisor plans - #95
Open
DivyamTalwar wants to merge 2 commits into
Open
DivyamTalwar wants to merge 2 commits into
DivyamTalwar wants to merge 2 commits into
Conversation
Plain plan output did not supply relation schemas. Candidate generation then guessed public, which can miss or mis-target a same-named relation in another schema. Request verbose plan metadata and decline candidates whose schema remains unresolved rather than inventing a target. Unit regressions fail on 9e41414 for a schema-less plan; the complete advisor race suite passes 20 repetitions and affected CLI tests pass. Add a uniquely owned PostgreSQL 16+ fixture with same-named public/non-public relations and hypopg validation; actual database validation is required before the PR. No query execution, dependency or model schema changes. The test database is created under a unique name and removed only after successful creation.
The PostgreSQL 16-18 CI images lack the optional server package required by the schema regression. Install HypoPG in those disposable containers and document the prerequisite rather than skipping the test or weakening its assertions. Production code and runtime dependencies are unchanged.
Contributor
Author
|
The PostgreSQL 16-18 CI failures came from the stock test containers lacking the HypoPG server package required by the new regression. Follow-up commit 24e97db installs that package in those CI containers and documents the prerequisite. The test remains mandatory on supported versions; no assertion or integration check was disabled. The committed-HEAD gate passes after the follow-up, and the new upstream run will exercise the fixture with its required extension. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
Closes #94.
The generic JSON plan omitted the resolved relation schema, while candidate generation guessed public. Non-public tables and same-named tables on another search path could therefore lose valid index recommendations. Request VERBOSE in the existing plan-only operation and decline schema-less nodes rather than inventing a target.
Verification
Verified commit:
24e97dbdb9c43d9f22b3b03dc935656a2005a4c0.Base regression fails for a schema-less plan being attributed to public.
Affected advisor/CLI suites and full race suite pass with the real dependencies.
bash scripts/gate.shpasses on this committed HEAD, including lint and six release targets.TestIntegration_advisorTargetsResolvedNonPublicSchemapasses without skips on disposable PostgreSQL 16 and 18 with hypopg. It exercises the actual read-only planner and distinguishes a non-public table from a same-named public control.Upstream CI passes all 19 checks on the follow-up commit. The PostgreSQL 16-18 jobs now install the HypoPG server package required by the integration fixture, rather than skipping its assertions.
Scope, risk and rollback
The advisor already requires PostgreSQL 16+ for generic plans. Missing schema is now treated as insufficient evidence. No runtime dependencies, model JSON fields or migrations change. CI installs the already-required HypoPG extension package for its disposable integration fixtures. No inspected query is executed. Revert the commit to restore the former plan options and fallback.
Checklist
scripts/gate.shpasses on committed HEAD; full race suite passes.