Обсуждение: pgsql: amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.
amcheck: Fix verify_heapam for tuples where xmin or xmax is 0. In such cases, get_xid_status() doesn't set its output parameter (the third argument), so we shouldn't fall through to code which will test the value of that parameter. There are five existing calls to get_xid_status(), three of which seem to already handle this case properly. This commit tries to fix the other two. If we're checking xmin and find that it is invalid (i.e. 0) just report that as corruption, similar to what's already done in the three cases that seem correct. If we're checking xmax and find that's invalid, that's fine: it just means that the tuple hasn't been updated or deleted. Thanks to Andres Freund and valgrind for finding this problem, and also to Andres for having a look at the patch. This bug seems to go all the way back to where verify_heapam was first introduced, but wasn't detected until recently, possibly because of the new test cases added for update chain verification. Back-patch to v14, where this code showed up. Discussion: http://postgr.es/m/CA+TgmoZAYzQZqyUparXy_ks3OEOfLD9-bEXt8N-2tS1qghX9gQ@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/e88754a1965c0f40a723e6e46d670cacda9e19bd Modified Files -------------- contrib/amcheck/verify_heapam.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
On Fri, Mar 24, 2023 at 8:13 AM Robert Haas <rhaas@postgresql.org> wrote: > If we're checking xmin and find that it is invalid (i.e. 0) just > report that as corruption, similar to what's already done in the > three cases that seem correct. If we're checking xmax and find > that's invalid, that's fine: it just means that the tuple hasn't > been updated or deleted. What about aborted speculative insertions? See heap_abort_speculative(), which directly sets the speculatively inserted heap tuple's xmin to InvalidTransactionId/zero. It probably does make sense to keep something close to this check -- it just needs to account for speculative insertions to avoid false positive reports of corruption. We could perform cross-checks against a tuple whose xmin is InvalidTransactionId/zero to verify that it really is from an aborted speculative insertion, to the extent that that's possible. For example, such a tuple can't be a heap-only tuple, and it can't have any xmax value other than InvalidTransactionId/zero. -- Peter Geoghegan