Re: Index AM API cleanup - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: Index AM API cleanup
Date
Msg-id f3d58fd2-3550-4d45-92f2-f988a938d659@dunslane.net
Whole thread Raw
List pgsql-hackers


On 2024-08-22 Th 2:28 PM, Mark Dilger wrote:

On Aug 22, 2024, at 1:36 AM, Alexandra Wang <alexandra.wang.oss@gmail.com> wrote:

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.
Thank you for the review, and the patches!


"make installcheck" for treeb is causing issues on my end. I can
investigate further if it’s not a problem for others.
The test module index AMs are not intended for use in any installed database, so 'make installcheck' is unnecessary.  A mere 'make check' should suffice.  However, if you want to run it, you can install the modules, edit postgresql.conf to add 'treeb' to shared_preload_libraries, restart the server, and run 'make installcheck'.   This is necessary for 'treeb' because it requests shared memory, and that needs to be done at startup.

The v18 patch set includes the changes your patches suggest, though I modified the approach a bit.  Specifically, rather than standardizing on '1.0.0' for the module versions, as your patches do, I went with '1.0', as is standard in other modules in neighboring directories.  The '1.0.0' numbering was something I had been using in versions 1..16 of this patch, and I only partially converted to '1.0' before posting v17, so sorry about that.  The v18 patch also has some whitespace fixes.

To address your comments about the noise in the test failures, v18 modifies the clone_tests.pl script to do a little more work translating the expected output to expect the module's AM name ("xash", "xtree", "treeb", or whatnot) beyond what that script did in v17.



Small addition to your clone script, taking into account the existence of alternative result files:


diff --git a/src/tools/clone_tests.pl b/src/tools/clone_tests.pl
index c1c50ad90b..d041f93f87 100755
--- a/src/tools/clone_tests.pl
+++ b/src/tools/clone_tests.pl
@@ -183,6 +183,12 @@ foreach my $regress (@regress)
    push (@dst_regress, "$dst_sql/$regress.sql");
    push (@src_expected, "$src_expected/$regress.out");
    push (@dst_expected, "$dst_expected/$regress.out");
+   foreach my $alt (1,2)
+   {
+       next unless -f "$src_expected/${regress}_$alt.out";
+       push (@src_expected, "$src_expected/${regress}_$alt.out");
+       push (@dst_expected, "$dst_expected/${regress}_$alt.out");
+   }
 }
 my @isolation = grep /\w+/, split(/\s+/, $isolation);
 foreach my $isolation (@isolation)


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: Enable data checksums by default
Next
From: Andrei Lepikhov
Date:
Subject: Re: allowing extensions to control planner behavior