Re: Index AM API cleanup

On 2024-08-22 Th 2:28 PM, Mark Dilger wrote:
>
>> On Aug 22, 2024, at 1:36 AM, Alexandra Wang<alexandra(dot)wang(dot)oss(at)gmail(dot)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