Stabilize regression tests against non-deterministic Heap Fetches#51
Merged
Conversation
The parallel Index Only Scan test was failing intermittently because Heap Fetches depends on the visibility map state, which varies with autovacuum activity between runs. Switch the bare EXPLAIN to portable_explain_analyze() and extend it with a regexp_replace that normalises "Heap Fetches: N" to a stable token, matching the existing treatment of row counts and buffer sizes. Also fix test ordering in the Makefile (interface before pg_track_optimizer so the reset counter starts at zero), remove an early DROP FUNCTION portable_explain_analyze that broke subsequent tests, and add missing includes for tuplestore.h and executor/instrument.h that were flagged by stricter builds.
Extend portable_explain_analyze() to normalise additional sources of version-specific output variation: - Strip "Index Searches: N" lines introduced in PG 18 so that a single expected file covers both PG 17 and PG 18+. This also lets us drop expected/join_filtering_1.out entirely, leaving one canonical file. - Normalise "actual rows=N.00" to "actual rows=N". PG 18 started printing row counts with two decimal places inside EXPLAIN ANALYZE output; the normalisation hides this from all callers of the helper. - Normalise "Heap Fetches: N" to mask the non-deterministic value that depends on visibility-map state at test time. Switch subplan.sql to route both EXPLAIN calls through portable_explain_analyze() so that the normalisation rules apply there too. Add "AND query LIKE 'EXPLAIN%'" to the tracking-result query so that only the inner EXPLAIN query appears in pg_track_optimizer(), not the portable_explain_analyze() wrapper itself. Update expected output files accordingly: - expected/subplan.out – PG 19+ (SubPlan expr_1 naming) - expected/subplan_1.out – PG 17–18 (SubPlan 1 naming) - expected/subplan_2.out – deleted; PG 16 is no longer supported - expected/join_filtering_1.out – deleted; consolidated into one file - expected/pg_track_optimizer.out / _1.out – updated for the portable_explain_analyze() change in the verify_test block Add a "Regression Test Expected Output Files" section to README.md documenting which file covers which PostgreSQL version and why.
Route all bare EXPLAIN ANALYZE calls in the test suite through portable_explain_analyze() so that version-specific output differences are normalised away in one place rather than duplicated across alternate expected files. Specific changes: - portable_explain_analyze() already stripped "actual rows=N.00", "Heap Fetches: N", and "Index Searches" lines. Apply it to the two remaining bare EXPLAIN ANALYZE calls in pg_track_optimizer.sql (pto_test and t1) that still exposed the rows=N.00 difference between PG 17 and PG 18+. Add AND query LIKE 'EXPLAIN%' to the t1 tracking check to exclude the portable_explain_analyze wrapper query from the results. - Replace the opening SELECT * FROM pg_track_optimizer_reset() with SELECT pg_track_optimizer_reset() >= 0 AS cleaned to mask a version-specific count (0 on PG 17, 1 on PG 18+) caused by the persistent DSM segment retaining the previous test's final reset call. - Delete expected/pg_track_optimizer_1.out: now that all EXPLAIN output is normalised, both PG 17 and PG 18+ produce identical output and a single expected file covers all supported versions. - Delete expected/join_filtering_1.out: portable_explain_analyze already filtered "Index Searches" lines, so the primary file was never matched and the _1 variant was doing all the work. Collapse to one file. - Delete expected/subplan_2.out: PG 16 is no longer supported. Rebuild expected/subplan_1.out (PG 17–18, SubPlan 1 naming) directly from actual test results to ensure correct column widths and trailing spaces rather than hand-writing it. - Update README to document the remaining alternate files and why they exist. After these changes only one variant remains: expected/subplan_1.out, kept solely because PG 19 renamed "SubPlan 1" to "SubPlan expr_1" — a content difference that output normalisation cannot bridge.
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.
The parallel Index Only Scan test was failing intermittently because Heap Fetches depends on the visibility map state, which varies with autovacuum activity between runs.
Switch the bare EXPLAIN to portable_explain_analyze() and extend it with a regexp_replace that normalises "Heap Fetches: N" to a stable token, matching the existing treatment of row counts and buffer sizes.
Also fix test ordering in the Makefile (interface before pg_track_optimizer so the reset counter starts at zero), remove an early DROP FUNCTION portable_explain_analyze that broke subsequent tests, and add missing includes for tuplestore.h and executor/instrument.h that were flagged by stricter builds.