Re: recovering from "found xmin ... from before relfrozenxid ..."

From: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
To: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
Cc: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>, MBeena Emerson <mbeena(dot)emerson(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>
Subject: Re: recovering from "found xmin ... from before relfrozenxid ..."
Date: 2020-08-28 09:55:19
Message-ID: CAE9k0Pkqpz90oHu8E-mWYRDSUuJQsQO4uukEA-CgOVDk3AX_LA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> > The tidcmp function can be removed, and ItemPointerCompare used
directly by qsort as:
> >
> > - qsort((void*) tids, ntids, sizeof(ItemPointerData),
tidcmp);
> > + qsort((void*) tids, ntids, sizeof(ItemPointerData),
> > + (int (*) (const void *, const void *))
ItemPointerCompare);
> >
>
> Will have a look into this.
>

We can certainly do this way, but I would still prefer having a comparator
function (tidcmp) here for the reasons that it makes the code look a bit
cleaner, it also makes us more consistent with the way the comparator
function argument is being passed to qsort at several other places in
postgres which kinda of increases the code readability and simplicity. For
e.g. there is a comparator function for gin that does the same thing as
tidcmp is doing here. See below:

static int
qsortCompareItemPointers(const void *a, const void *b)
{
int res = ginCompareItemPointers((ItemPointer) a, (ItemPointer)
b);

/* Assert that there are no equal item pointers being sorted */
Assert(res != 0);
return res;
}

In this case as well, it could have been done the way you are suggesting,
but it seems like writing a small comparator function with the prototype
that qsort accepts looked like a better option. Considering this, I am just
leaving this as-it-is. Please let me know if you feel the other way round.

> > The header comment for function find_tids_one_page should state the
requirement that the tids array must be sorted.
> >
>
> Okay, will add a comment for this.
>

Added a comment for this in the attached patch.

Please have a look into the attached patch for the changes and let me know
for any other concerns. Thank you.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

Attachment Content-Type Size
v10-0001-Add-contrib-pg_surgery-to-perform-surgery-on-a-damag.patch text/x-patch 28.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2020-08-28 10:13:29 Re: New default role- 'pg_read_all_data'
Previous Message osumi.takamichi@fujitsu.com 2020-08-28 09:29:16 RE: extension patch of CREATE OR REPLACE TRIGGER