Re: Index AM API cleanup

From: Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>
To: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Andrew Dunstan <andrew(dot)dunstan(at)enterprisedb(dot)com>, Alex Wang <alex(dot)wang(at)enterprisedb(dot)com>
Subject: Re: Index AM API cleanup
Date: 2024-08-22 08:36:31
Message-ID: CAK98qZ3fuCv06uhh=bYN=C4ktwA+LM19FzJ-AJHjDHxn6c1k9A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Mark,

On Wed, Aug 21, 2024 at 2:25 PM Mark Dilger
<mark(dot)dilger(at)enterprisedb(dot)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

Attachment Content-Type Size
0001-Fixup-make-check-for-xash-and-xtree.patch application/octet-stream 1.9 KB
0002-Fixup-treeb-compilation.patch application/octet-stream 975 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2024-08-22 08:49:22 Re: Disallow USING clause when altering type of generated column
Previous Message Frédéric Yhuel 2024-08-22 08:10:53 pgstattuple: fix free space calculation