8000 Avoid scribbling of VACUUM options · postgres/postgres@d187cab · GitHub
[go: up one dir, main page]

Skip to content

Commit d187cab

Browse files
committed
Avoid scribbling of VACUUM options
This fixes two issues with the handling of VacuumParams in vacuum_rel(). This code path has the idea to change the passed-in pointer of VacuumParams for the "truncate" and "index_cleanup" options for the relation worked on, impacting the two following scenarios where incorrect options may be used because a VacuumParams pointer is shared across multiple relations: - Multiple relations in a single VACUUM command. - TOAST relations vacuumed with their main relation. The problem is avoided by providing to the two callers of vacuum_rel() copies of VacuumParams, before the pointer is updated for the "truncate" and "index_cleanup" options. The refactoring of the VACUUM option and parameters done in 0d83138 did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with b84dbc8 for "truncate" and a96c41f for "index_cleanup" that have been added a couple of years after the initial refactoring. HEAD will be improved with a different patch that hardens the uses of VacuumParams across the tree. This cannot be backpatched as it introduces an ABI breakage. The backend portion of the patch has been authored by Nathan, while I have implemented the tests. The tests rely on injection points to check the option values, making them faster, more reliable than the tests originally proposed by Shihao, and they also provide more coverage. This part can only be backpatched down to v17. Reported-by: Shihao Zhong <zhong950419@gmail.com> Author: Nathan Bossart <nathandbossart@gmail.com> Co-authored-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com Backpatch-through: 13
1 parent 87c8ed3 commit d187cab

File tree

1 file changed

+16
-4
lines changed

1 file changed

+16
-4
lines changed

src/backend/commands/vacuum.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,15 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
619619

620620
if (params->options & VACOPT_VACUUM)
621621
{
622-
if (!vacuum_rel(vrel->oid, vrel->relation, params, bstrategy))
622+
VacuumParams params_copy;
623+
624+
/*
625+
* vacuum_rel() scribbles on the parameters, so give it a copy
626+
* to avoid affecting other relations.
627+
*/
628+
memcpy(&params_copy, params, sizeof(VacuumParams));
629+
630+
if (!vacuum_rel(vrel->oid, vrel->relation, &params_copy, bstrategy))
623631
continue;
624632
}
625633

@@ -1993,9 +2001,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
19932001
Oid save_userid;
19942002
int save_sec_context;
19952003
int save_nestlevel;
2004+
VacuumParams toast_vacuum_params;
19962005

19972006
Assert(params != NULL);
19982007

2008+
/*
2009+
* This function scribbles on the parameters, so make a copy early to
2010+
* avoid affecting the TOAST table (if we do end up recursing to it).
2011+
*/
2012+
memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
2013+
19992014
/* Begin a transaction for vacuuming this relation */
20002015
StartTransactionCommand();
20012016

@@ -2258,10 +2273,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
22582273
*/
22592274
if (toast_relid != InvalidOid)
22602275
{
2261-
VacuumParams toast_vacuum_params;
2262-
22632276
/* force VACOPT_PROCESS_MAIN so vacuum_rel() processes it */
2264-
memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
22652277
toast_vacuum_params.options |= VACOPT_PROCESS_MAIN;
22662278

22672279
vacuum_rel(toast_relid, NULL, &toast_vacuum_params, bstrategy);

0 commit comments

Comments
 (0)
0