> > 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.
