Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix swap of relfrozenxid, relfrozenxid and relallvisible #377

Merged
merged 3 commits into from
May 21, 2024
Merged

Fix swap of relfrozenxid, relfrozenxid and relallvisible #377

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bbbd008
fix relfrozenxid and relminmxid on table data files swap
bashtanov Nov 1, 2017
1b5daa9
Fix typos and don't use PG_VERSION_NUM condition
za-arthur Jan 5, 2024
3792bad
Rename swaptempxid to fronzenxid, add minmxid
za-arthur May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions lib/repack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,7 @@ swap_heap_or_index_files(Oid r1, Oid r2)
Form_pg_class relform1,
relform2;
Oid swaptemp;
TransactionId swaptempxid;
za-arthur marked this conversation as resolved.
Show resolved Hide resolved
CatalogIndexState indstate;

/* We need writable copies of both pg_class tuples. */
Expand Down Expand Up @@ -1225,20 +1226,25 @@ swap_heap_or_index_files(Oid r1, Oid r2)
relform1->reltoastrelid = relform2->reltoastrelid;
relform2->reltoastrelid = swaptemp;

/* set rel1's frozen Xid to larger one */
if (TransactionIdIsNormal(relform1->relfrozenxid))
/*
* Swap relfrozenxid and relminmxid, as they must be consistent with the data
*/
if (relform1->relkind != RELKIND_INDEX)
{
if (TransactionIdFollows(relform1->relfrozenxid,
relform2->relfrozenxid))
relform1->relfrozenxid = relform2->relfrozenxid;
else
relform2->relfrozenxid = relform1->relfrozenxid;
swaptempxid = relform1->relfrozenxid;
relform1->relfrozenxid = relform2->relfrozenxid;
relform2->relfrozenxid = swaptempxid;

swaptempxid = relform1->relminmxid;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use a MultiXactId variable instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used TransactionId mainly because relminmxid has that type, regardless of the comment in pg_class.h, stating that it is a MultiXactId.
I wonder why pg_class.h doesn't use MultiXactId for relminmxid.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why pg_class.h doesn't use MultiXactId for relminmxid.

I think MultiXactId doesn't have the corresponding SQL data type unlike TransactionId. Data types that can be used in system catalog are somewhat limited. It makes sense to use MultiXactId internally since we use it in C functions.

relform1->relminmxid = relform2->relminmxid;
relform2->relminmxid = swaptempxid;
}

/* swap size statistics too, since new rel has freshly-updated stats */
{
int32 swap_pages;
float4 swap_tuples;
int32 swap_allvisible;

swap_pages = relform1->relpages;
relform1->relpages = relform2->relpages;
Expand All @@ -1247,6 +1253,10 @@ swap_heap_or_index_files(Oid r1, Oid r2)
swap_tuples = relform1->reltuples;
relform1->reltuples = relform2->reltuples;
relform2->reltuples = swap_tuples;

swap_allvisible = relform1->relallvisible;
relform1->relallvisible = relform2->relallvisible;
relform2->relallvisible = swap_allvisible;
}

indstate = CatalogOpenIndexes(relRelation);
Expand Down