Horton
Menu

Re: Index AM API cleanup

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Andrew Dunstan <andrew.dunstan@enterprisedb.com>, Alex Wang <alex.wang@enterprisedb.com>
Date: 2024-08-26T12:21:26Z
Lists: pgsql-hackers 
On 21.08.24 21:25, Mark Dilger wrote:
> The next twenty patches are a mix of fixes of various layering
> violations, such as not allowing non-core index AMs from use in replica
> identity full, or for speculative insertion, or for foreign key
> constraints, or as part of merge join; with updates to the "treeb" code
> as needed.  The changes to "treeb" are broken out so that they can also
> easily be excluded from whatever gets committed.

I made a first pass through this patch set.  I think the issues it aims 
to address are mostly legitimate.  In a few cases, we might need some 
more discussion and perhaps will end up slicing the APIs a bit 
differently.  The various patches that generalize the strategy numbers 
appear to overlap with things being discussed at [0], so we should see 
that the solution covers all the use cases.

[0]: 
https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

To make a dent, I picked out something that should be mostly harmless: 
Stop calling directly into _bt_getrootheight() (patch 0004).  I think 
this patch is ok, but I might call the API function amgettreeheight 
instead of amgetrootheight.  The former seems more general.

Also, a note for us all in this thread, changes to the index AM API need 
updates to the corresponding documentation in doc/src/sgml/indexam.sgml.

I notice that _bt_getrootheight() is called only to fill in the 
IndexOptInfo tree_height field, which is only used by btcostestimate(), 
so in some sense this is btree-internal data.  But making it so that 
btcostestimate() calls _bt_getrootheight() directly to avoid all that 
intermediate business seems too complicated, and there was probably a 
reason that the cost estimation functions don't open the index.

Interestingly, the cost estimation functions for gist and spgist also 
look at the tree_height field but nothing ever fills it on.  So with 
your API restructuring, someone could provide the missing API functions 
for those index types.  Might be interesting.

That said, there might be value in generalizing this a bit.  If you look 
at the cost estimation functions in pgvector (hnswcostestimate() and 
ivfflatcostestimate()), they both have this pattern that 
btcostestimate() tries to avoid: They open the index, look up some 
number, close the index, then make a cost estimate computation with the 
number looked up.  So another idea would be to generalize the 
tree_height field to some "index size data" or even "internal data for 
cost estimation".  This wouldn't need to change the API much, since 
these are all just integer values, but we'd label the functions and 
fields a bit differently.

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