Horton
Menu

Re: Index AM API cleanup

Alexandra Wang <alexandra.wang.oss@gmail.com>

View thread
From: Alexandra Wang <alexandra.wang.oss@gmail.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Andrew Dunstan <andrew.dunstan@enterprisedb.com>, Alex Wang <alex.wang@enterprisedb.com>
Date: 2024-08-22T08:36:31Z
Lists: pgsql-hackers

Attachments

Hi Mark,

On Wed, Aug 21, 2024 at 2:25 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
>
>
> For validation purposes, the first patch creates shallow copies of hash and btree named "xash" and "xtree" and introduces some infrastructure to run the src/test/regress and src/test/isolation tests against them without needing to duplicate those tests.  Numerous failures like "unexpected non-btree AM" can be observed in the test results.
>
> Also for validation purposes, the second patch creates a deep copy of btree named "treeb" which uses modified copies of the btree implementation rather than using the btree implementation by reference.  This allows for more experimentation, but might be more code than the community wants.  Since this is broken into its own patch, it can be excluded from what eventually gets committed.  Even if we knew a priori that this "treeb" test would surely never be committed, it still serves to help anybody reviewing the patch series to experiment with those other changes without having to construct such a test index AM individually.

Thank you for providing an infrastructure that allows modules to test
by reference and re-use existing regression and isolation tests. I
believe this approach ensures great coverage for the API cleanup.
I was very excited to compare the “make && make check” results in the
test modules - ‘xash,’ ‘xtree,’ and ‘treeb’ - before and after the
series of AM API fixes. Here are my results:

Before the fixes:
- xash: # 20 of 223 tests failed.
- xtree: # 95 of 223 tests failed.
- treeb: # 47 of 223 tests failed.

After the fixes:
- xash: # 21 of 223 tests failed.
- xtree: # 58 of 223 tests failed.
- treeb: # 58 of 223 tests failed.

I expected the series of fixes to eliminate all failed tests, but that
wasn’t the case. It's nice to see the failures for ‘xtree’ have significantly
decreased as the ‘unexpected non-btree AM’ errors have been resolved.
I noticed some of the remaining test failures are due to trivial index
name changes, like hash -> xash and btree -> treeb.

If we keep xtree and xash for testing, is there a way to ensure all
tests pass instead of excluding them from "make check-world"? On that
note, I ran ‘make check-world’ on the final patch, and everything
looks good.

I had to make some changes to the first two patches in order to run
"make check" and compile the treeb code on my machine. I’ve attached
my changes.

"make installcheck" for treeb is causing issues on my end. I can
investigate further if it’s not a problem for others.

Best,
Alex

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