[PATCH] Handle SK_SEARCHNULL and SK_SEARCHNOTNULL in HeapKeyTest

Started by Sven Klemmalmost 2 years ago5 messageshackers
Jump to latest
#1Sven Klemm
sven@timescale.com

Hello,

This patches changes the HeapKeyTest macro to add handling for SK_SEARCHNULL
and SK_SEARCHNOTNULL. While currently no core codes uses these ScanKey flags
it would be useful for extensions if it was supported so extensions
dont have to implement
handling for those by themself.

--
Regards, Sven Klemm

Attachments:

0001-Handle-SK_SEARCHNULL-and-SK_SEARCHNOTNULL-in-HeapKey.patchtext/x-patch; charset=US-ASCII; name=0001-Handle-SK_SEARCHNULL-and-SK_SEARCHNOTNULL-in-HeapKey.patchDownload+8-2
#2Aleksander Alekseev
aleksander@timescale.com
In reply to: Sven Klemm (#1)
Re: [PATCH] Handle SK_SEARCHNULL and SK_SEARCHNOTNULL in HeapKeyTest

Hi,

This patches changes the HeapKeyTest macro to add handling for SK_SEARCHNULL
and SK_SEARCHNOTNULL. While currently no core codes uses these ScanKey flags
it would be useful for extensions if it was supported so extensions
dont have to implement
handling for those by themself.

As I recall, previously it was argued that changes like this should
have some use within the core [1]https://commitfest.postgresql.org/42/4180/.

Can you think of any such use?

[1]: https://commitfest.postgresql.org/42/4180/

--
Best regards,
Aleksander Alekseev

#3Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Aleksander Alekseev (#2)
Re: [PATCH] Handle SK_SEARCHNULL and SK_SEARCHNOTNULL in HeapKeyTest

On Mon, 1 Jul 2024 at 15:48, Aleksander Alekseev
<aleksander@timescale.com> wrote:

As I recall, previously it was argued that changes like this should
have some use within the core [1].

I don't see that argument anywhere in the thread honestly. I did see
heiki asking why it would be useful for extensions, but that was
answered there.

#4Aleksander Alekseev
aleksander@timescale.com
In reply to: Jelte Fennema-Nio (#3)
Re: [PATCH] Handle SK_SEARCHNULL and SK_SEARCHNOTNULL in HeapKeyTest

Hi,

As I recall, previously it was argued that changes like this should
have some use within the core [1].

I don't see that argument anywhere in the thread honestly. I did see
heiki asking why it would be useful for extensions, but that was
answered there.

The referred patch was rejected at first because it didn't modify
nodeSeqScan.c to make use of the change within the core.

I'm not saying this is necessarily applicable to this particular patch
or that this is a general rule though.

--
Best regards,
Aleksander Alekseev

#5Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Aleksander Alekseev (#4)
Re: [PATCH] Handle SK_SEARCHNULL and SK_SEARCHNOTNULL in HeapKeyTest

On Tue, 2 Jul 2024 at 10:15, Aleksander Alekseev
<aleksander@timescale.com> wrote:

The referred patch was rejected at first because it didn't modify
nodeSeqScan.c to make use of the change within the core.

I guess we interpret Heikis email differently. I read it as: "If this
improves performance, then let's also start using it in core. If not,
why do extensions need it?" And I think you quite clearly explained
that even if perf is not better, then the usability for extensions
that don't want to use SPI is better.

I don't think Heiki meant his response as not using it in core being a
blocker for the patch. But maybe my interpretation of his response is
incorrect.