pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Avoid scribbling of VACUUM options 6+ messages / 1 participants [nested] [flat]
* pgsql: Avoid scribbling of VACUUM options @ 2025-06-25 01:05 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 6+ messages in thread From: Michael Paquier @ 2025-06-25 01:05 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 0d831389749a did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with b84dbc8eb80b for "truncate" and a96c41feec6b 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 Branch ------ REL_15_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/354944663df38a360f8519814658a10cb5c3d04a Modified Files -------------- src/backend/commands/vacuum.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Avoid scribbling of VACUUM options @ 2025-06-25 01:05 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 6+ messages in thread From: Michael Paquier @ 2025-06-25 01:05 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 0d831389749a did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with b84dbc8eb80b for "truncate" and a96c41feec6b 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 Branch ------ REL_14_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/c079ba3fc2a4ed4801a0adf55cbbf69c961b1ba4 Modified Files -------------- src/backend/commands/vacuum.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Avoid scribbling of VACUUM options @ 2025-06-25 01:05 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 6+ messages in thread From: Michael Paquier @ 2025-06-25 01:05 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 0d831389749a did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with b84dbc8eb80b for "truncate" and a96c41feec6b 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 Branch ------ REL_13_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/65c3223f93a0e5dc4ae1f3ec5fd48455b2da8d86 Modified Files -------------- src/backend/commands/vacuum.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Avoid scribbling of VACUUM options @ 2025-06-25 01:05 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 6+ messages in thread From: Michael Paquier @ 2025-06-25 01:05 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 0d831389749a did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with b84dbc8eb80b for "truncate" and a96c41feec6b 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 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/661643dedad97f8b924991fdc739b1f47b0fb60b Modified Files -------------- src/backend/commands/vacuum.c | 39 ++++++- src/test/modules/injection_points/Makefile | 2 +- .../modules/injection_points/expected/vacuum.out | 122 +++++++++++++++++++++ src/test/modules/injection_points/meson.build | 1 + src/test/modules/injection_points/sql/vacuum.sql | 47 ++++++++ 5 files changed, 206 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Avoid scribbling of VACUUM options @ 2025-06-25 01:05 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 6+ messages in thread From: Michael Paquier @ 2025-06-25 01:05 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 0d831389749a did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with b84dbc8eb80b for "truncate" and a96c41feec6b 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 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/d187cabddadcbea2033d172e738edbd65b5a3f8d Modified Files -------------- src/backend/commands/vacuum.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* pgsql: Avoid scribbling of VACUUM options @ 2025-06-25 01:05 Michael Paquier <michael@paquier.xyz> 0 siblings, 0 replies; 6+ messages in thread From: Michael Paquier @ 2025-06-25 01:05 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org 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 0d831389749a did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with b84dbc8eb80b for "truncate" and a96c41feec6b 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 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/2e0b5d252b10b790abdee9dec202a306006037c2 Modified Files -------------- src/backend/commands/vacuum.c | 39 +++++++- src/test/modules/injection_points/Makefile | 2 +- .../modules/injection_points/expected/vacuum.out | 103 +++++++++++++++++++++ src/test/modules/injection_points/meson.build | 1 + src/test/modules/injection_points/sql/vacuum.sql | 35 +++++++ 5 files changed, 175 insertions(+), 5 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-06-25 01:05 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-06-25 01:05 pgsql: Avoid scribbling of VACUUM options Michael Paquier <michael@paquier.xyz> 2025-06-25 01:05 pgsql: Avoid scribbling of VACUUM options Michael Paquier <michael@paquier.xyz> 2025-06-25 01:05 pgsql: Avoid scribbling of VACUUM options Michael Paquier <michael@paquier.xyz> 2025-06-25 01:05 pgsql: Avoid scribbling of VACUUM options Michael Paquier <michael@paquier.xyz> 2025-06-25 01:05 pgsql: Avoid scribbling of VACUUM options Michael Paquier <michael@paquier.xyz> 2025-06-25 01:05 pgsql: Avoid scribbling of VACUUM options Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox