Phoebe: simplify reconcile index test to the one structural invariant (stop the regress)#21
Closed
hhuuggoo wants to merge 1 commit into
Closed
Phoebe: simplify reconcile index test to the one structural invariant (stop the regress)#21hhuuggoo wants to merge 1 commit into
hhuuggoo wants to merge 1 commit into
Conversation
TestIntegration_ReconcileDeleteCanUseWindowStartIndex existed to prove ONE structural invariant: the reconcile DELETE's window_start-only predicate CAN be served by rated_usage_window_start_ix (not a seqscan, not the auth-leading composite). The test had accreted a vacuous soft signal and an over-narrow assertion that flaked across plan renderings. This is test-quality only. - Keep exactly one hard assertion: under enable_seqscan=off, the EXPLAIN'd reconcile DELETE must reach rated_usage through rated_usage_window_start_ix. Match the index by NAME across all node forms (Index Scan / Index Only Scan via "using …", and Bitmap Index Scan via "on …"). On a right-sized population PG 16 actually picks a Bitmap Index Scan, which the old "using …"-only match would have failed on — that was the flake. - Delete the default-cost soft-signal (t.Logf) block: planner cost-preference is version/GUC-fragile and its failure mode is a slow reconcile, not a wrong bill, so it gated nothing and proved nothing. - Right-size the population from 10k rows (240 windows) to 240 rows (24 windows), which only backed the deleted default-cost assertion. The remaining ANALYZE'd set is enough for the predicate to be a real index-served slice under seqscan-off. - Rewrite the comment to state honestly what it proves: index USABILITY for the reconcile predicate, not a planner-cost-preference or performance guarantee. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Landed on main via the squashed rating merge (3b22908). Closing the stack. |
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.
Cleans up
TestIntegration_ReconcileDeleteCanUseWindowStartIndexto prove the one structural invariant it exists for — the reconcile DELETE'swindow_start-only predicate can be served byrated_usage_window_start_ix(not a seqscan, not the auth-leading composite) — and drops the accreted parts that generated review findings without gating anything.enable_seqscan=off, the EXPLAIN'd reconcile DELETE must reachrated_usagethroughrated_usage_window_start_ix. Matches the index by NAME across all node forms —Index Scan/Index Only Scan(using …) andBitmap Index Scan(on …). On the right-sized population PG 16 actually renders a Bitmap Index Scan, which the previoususing …-only match would have failed on; that over-narrow match was the flake.t.Logf) block: planner cost-preference is version/GUC-fragile, its failure mode is a slow reconcile (not a wrong bill), and it gated nothing.Test-file-only (
internal/rating/store_integration_test.go); no production code, migrations, or other tests touched.Contracts
No production or contract change — test-quality only (one EXPLAIN integration test made honest and non-flaky).