Re: pgsql: amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.
Date: 2023-03-27 20:17:32
Message-ID: CA+Tgmobi9X1UX8sfLQ4SS5u4TfJgg3-v5CX2HDs2Z73oHGr_Aw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Mon, Mar 27, 2023 at 2:34 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> > Since this was back-patched, I think it's probably better to just
> > remove the error. We can introduce new validation if we want, but that
> > should probably be master-only.
>
> That makes sense.

Patch attached.

> I don't think that it's particularly likely that having refined
> aborted speculative insertion amcheck coverage will make a critical
> difference to any user, at any time. But "amcheck as documentation of
> the on-disk format" is reason enough to have it.

Sure, if someone feels like writing the code. I have to admit that I
have mixed feelings about this whole direction. In concept, I agree
with you entirely: a fringe benefit of having checks that tell us
whether or not a page is valid is that it helps to make clear what
page states we think are valid. In practice, however, the point you
raise in your first sentence weighs awfully heavily with me. Spending
a lot of energy on checks that are unlikely to catch practical
problems feels like it may not be the best use of time. I'm not sure
exactly where to draw the line, but it seems highly likely to be that
there are things we could deduce about the page that wouldn't be worth
the effort. For example, would we bother checking that a tuple with an
in-progress xmin does not have a smaller natts value than a tuple with
a committed xmin? Or that natts values are non-decreasing across a HOT
chain? I suspect there are even more obscure examples of things that
should be true but might not really be worth worrying about in the
code.

--
Robert Haas
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
0001-amcheck-In-verify_heapam-allows-tuples-with-xmin-0.patch application/octet-stream 1.2 KB

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Geoghegan 2023-03-27 20:51:38 Re: pgsql: amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.
Previous Message Daniel Gustafsson 2023-03-27 19:47:19 pgsql: doc: Fix XML_CATALOG_FILES env var for Apple Silicon machines

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2023-03-27 20:47:22 Re: running logical replication as the subscription owner
Previous Message Jeff Davis 2023-03-27 19:21:15 Re: Non-superuser subscription owners