heapgettup() with NoMovementScanDirection unused in core?

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: heapgettup() with NoMovementScanDirection unused in core?
Date: 2023-01-25 00:55:23
Message-ID: CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

David Rowley and I were discussing how to test the
NoMovementScanDirection case for heapgettup() and heapgettup_pagemode()
in [1] (since there is not currently coverage). We are actually
wondering if it is dead code (in core).

This is a link to the code in question on github in [2] (side note: is
there a more evergreen way to do this that doesn't involve pasting a
hundred lines of code into this email? You need quite a few lines of
context for it to be clear what code I am talking about.)

standard_ExecutorRun() doesn't run ExecutePlan() if scan direction is no
movement.

if (!ScanDirectionIsNoMovement(direction))
{
...
ExecutePlan(estate,
queryDesc->planstate,
}

And other users of heapgettup() through table_scan_getnextslot() and the
like all seem to pass ForwardScanDirection as the direction.

A skilled code archaeologist brought our attention to adbfab119b308a
which appears to remove the only users in the core codebase calling
heapgettup() and heapgettup_pagemode() with NoMovementScanDirection (and
those users were not themselves used).

Perhaps we can remove support for NoMovementScanDirection with
heapgettup()? Unless someone knows of a good use case for a table AM to
do this?

- Melanie

[1] https://www.postgresql.org/message-id/CAAKRu_ZyiXwWS1WXSZneoy%2BsjoH_%2BF5UhO-1uFhyi-u0d6z%2BfA%40mail.gmail.com
[2] https://github.com/postgres/postgres/blob/master/src/backend/access/heap/heapam.c#L656

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Melanie Plageman 2023-01-25 00:58:43 Re: heapgettup refactoring
Previous Message Tom Lane 2023-01-25 00:42:06 Re: 011_crash_recovery.pl intermittently fails