Horton
Menu

Re: Index AM API cleanup

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Andrew Dunstan <andrew@dunslane.net>, Mark Dilger <mark.dilger@enterprisedb.com>, Kirill Reshke <reshkekirill@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Alex Wang <alex.wang@enterprisedb.com>
Date: 2024-11-27T12:57:57Z
Lists: pgsql-hackers 
On 26.11.24 15:27, Andrew Dunstan wrote:
> On 2024-11-19 Tu 6:26 PM, Mark Dilger wrote:
>>> On Nov 16, 2024, at 9:10 AM, Kirill Reshke <reshkekirill@gmail.com> 
>>> wrote:
>>>
>>> Hi! Can we please have a rebased version of this patch series?
>> Sorry for the delay, and thanks for your interest.  I will try to get 
>> around to rebasing in the next few days.
> 
> beat you to it :-)

Thanks for that.

I'm content to move forward with the approach of mapping to 
RowCompareType, as we had already discussed.

I think, however, that we should rename RowCompareType.  Otherwise, it's 
just going to be confusing forevermore.  I suggest to rename it simply 
to CompareType.

What I really don't like is that there is ROWCOMPARE_INVALID *and* 
ROWCOMPARE_NONE.  No one is going to understand that and keep it 
straight.  I also don't really see a reason for having both.  I tried 
removing one of them, and the only thing that failed was the new code in 
AlterOpFamilyAdd() that tries to use strategy_get_rctype() to detect 
valid operator numbers.  But that's new code that is not essential to 
the purpose of this patch series.  So I strongly suggest that we just 
have ROWCOMPARE_INVALID (to mirror InvalidStrategy) with value 0.  If 
there is a reason to have a _NONE that is separate from that, we need to 
think of a different interface.

We should make sure that gist is properly integrated into this 
framework, give that we already have strategy number mapping logic 
there.  The current patch series does not provide mapping functions for 
gist.  We should add those.  They should make use of 
GistTranslateStratnum() (and we might need to add an inverse function). 
The problem is that gist and GistTranslateStratnum() also need the 
opclass, because the strategy numbers are not fixed for gist.  So the 
amtranslatestrategy and amtranslaterctype callbacks probably need to 
expanded to cover that.

It might be that spgist has a similar requirement, I'm not sure.  The 
code in spgutils.c raises some doubts about what it's doing.  I'm 
curious why this patch set provides an implementation for spgist but not 
gist.  Is there anything interesting about spgist for this purpose?

I'm going to try to code up the gist support on top of this patch set to 
make sure that it will fit well.  I'll report back.

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Relax ordering-related hardcoded btree requirements in planning

  2. Support non-btree indexes in get_actual_variable_range()

  3. Convert PathKey to use CompareType

  4. Generalize index support in network support function

  5. Update a code comment

  6. Allow non-btree unique indexes for matviews

  7. Allow non-btree unique indexes for partition keys

  8. Add some opfamily support functions to lsyscache.c

  9. Simplify and generalize PrepareSortSupportFromIndexRel()

  10. Generalize hash and ordering support in amapi

  11. Drop opcintype from index AM strategy translation API

  12. Allow non-btree speculative insertion indexes

  13. Support non-btree indexes for foreign keys

  14. Integrate GistTranslateCompareType() into IndexAmTranslateCompareType()

  15. Convert strategies to and from compare types

  16. Move CompareType to separate header file

  17. Add get_opfamily_name() function

  18. Rename GistTranslateStratnum() to GistTranslateCompareType()

  19. Change gist stratnum function to use CompareType

  20. Rename RowCompareType to CompareType

  21. Improve slightly misleading internal error message

  22. Track scan reversals in MergeJoin

  23. Track sort direction in SortGroupClause

  24. Add stratnum GiST support function

  25. Add amgettreeheight index AM API routine