From 661d9837ba34552fa301dc17b1fd74dd833aec72 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 7 Sep 2023 17:35:46 -0400 Subject: [PATCH v4 1/3] Clarify usage of heap_page_prune parameter off_loc heap_page_prune() takes an optional parameter, off_loc, where it saves the current offset while pruning. Vacuum passes the address of vacrel->offnum, which is used by vacuum_error_callback() to populate the error context message. On-access pruning has no such callback mechanism, so it passes NULL. When off_loc is not provided the error message context simply omits the offset. Because vacrel->offnum is specifically used by vacuum_error_callback(), it is required that it be passed to heap_page_prune() for the error message to be correctly populated. Clarify this requirement in a comment in lazy_scan_prune() before calling heap_page_prune(). While we are at it, provide some additional detail about off_loc in heap_page_prune()'s function header comment. Discussion: https://postgr.es/m/CA%2BTgmoZ8E7DXzAZkV%3DRF9tfJkQFH%3Da3KkjQhYNY9W26Jh7AN%2Bw%40mail.gmail.com --- src/backend/access/heap/pruneheap.c | 7 +++++-- src/backend/access/heap/vacuumlazy.c | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 18193efa23..392b54f093 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -207,8 +207,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer) * Sets *nnewlpdead for caller, indicating the number of items that were * newly set LP_DEAD during prune operation. * - * off_loc is the offset location required by the caller to use in error - * callback. + * off_loc is the current offset into the line pointer array while pruning. + * This is used by vacuum to populate the error context message. On-access + * pruning has no such callback mechanism for populating the error context, so + * it passes NULL. When provided by the caller, off_loc is set exclusively by + * heap_page_prune(). * * Returns the number of tuples deleted from the page during this call. */ diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 1a05adfa61..102cc97358 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1586,6 +1586,10 @@ retry: * deleted from the table. It should not be confused with lpdead_items; * lpdead_items's final value can be thought of as the number of tuples * that were deleted from indexes. + * + * vacuum_error_callback() looks specifically at vacrel->offnum for the + * current offset when populating the error context message, so it is + * imperative that we pass its location to heap_page_prune. */ tuples_deleted = heap_page_prune(rel, buf, vacrel->vistest, &nnewlpdead, -- 2.37.2