pg_dump: Adjust reltuples from 0 to -1 for dumps of older versions. #8
Conversation
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
timothy-e
approved these changes
May 22, 2025
timothy-e
left a comment
timothy-e
left a comment
There was a problem hiding this comment.
LGTM! At first I thought you messed up the author info, but then I realized you are the author! Congrats on committing to PG 🥳
Before v14, a reltuples value of 0 was ambiguous: it could either mean the relation is empty, or it could mean that it hadn't yet been vacuumed or analyzed. (Commit 3d351d9 taught v14 and newer to use -1 for the latter case.) This ambiguity allegedly can cause the planner to choose inefficient plans after restoring to v18 or newer. To fix, let's just dump reltuples as -1 in that case. This will cause some truly empty tables to be seen as not-yet-processed, but that seems unlikely to cause too much trouble in practice. Note that we could alternatively teach pg_restore_relation_stats() to translate reltuples based on the version argument, but since that function doesn't exist until v18, there's no particular advantage to that approach. That is, there's no chance of restoring stats dumped from a pre-v14 server to another pre-v14 server. Per discussion, the current policy is to fix pre-v18 behavior differences during export and everything else during import. Commit 9879105 fixed a similar problem for vacuumdb by removing the check for reltuples != 0. Presumably we could reinstate that check now, but I've chosen to leave it in place in case reltuples isn't accurate. As before, processing some empty tables seems relatively harmless. Author: Hari Krishna Sunder <hari.db.pg@gmail.com> Reviewed-by: Jeff Davis <pgsql@j-davis.com> Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://postgr.es/m/CAAeiqZ0o2p4SX5_xPcuAbbsmXjg6MJLNuPYSLUjC%3DWh-VeW64A%40mail.gmail.com (cherry picked from commit 5d6eac8)
Author
Done. Thanks for catching this. |
jasonyb
approved these changes
May 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Import upstream postgres commit 5d6eac8.
Phorge Revision: D44218
Before v14, a reltuples value of 0 was ambiguous: it could either mean the relation is empty, or it could mean that it hadn't yet been vacuumed or analyzed. (Commit 3d351d9 taught v14 and newer to use -1 for the latter case.) This ambiguity allegedly can cause the planner to choose inefficient plans after restoring to v18 or newer. To fix, let's just dump reltuples as -1 in that case. This will cause some truly empty tables to be seen as not-yet-processed, but that seems unlikely to cause too much trouble in practice.
Note that we could alternatively teach pg_restore_relation_stats() to translate reltuples based on the version argument, but since that function doesn't exist until v18, there's no particular advantage to that approach. That is, there's no chance of restoring stats dumped from a pre-v14 server to another pre-v14 server. Per discussion, the current policy is to fix pre-v18 behavior differences during export and everything else during import.
Commit 9879105 fixed a similar problem for vacuumdb by removing the check for reltuples != 0. Presumably we could reinstate that check now, but I've chosen to leave it in place in case reltuples isn't accurate. As before, processing some empty tables seems relatively harmless.
Author: Hari Krishna Sunder hari.db.pg@gmail.com
Reviewed-by: Jeff Davis pgsql@j-davis.com
Reviewed-by: Corey Huinker corey.huinker@gmail.com
Discussion: https://postgr.es/m/CAAeiqZ0o2p4SX5_xPcuAbbsmXjg6MJLNuPYSLUjC%3DWh-VeW64A%40mail.gmail.com
(cherry picked from commit 5d6eac8)