Re: Index AM API cleanup

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(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 18:28:39
Message-ID: 3C52C6D1-8D96-4935-844A-B6DBF2FD9A30@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 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.

Attachment Content-Type Size
v18.tar.bz2 application/x-bzip2 220.2 KB
unknown_filename text/plain 97 bytes

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2024-08-22 18:31:11 Re: Partial aggregates pushdown
Previous Message Jeff Davis 2024-08-22 18:00:54 Refactor: allow pg_strncoll(), etc., to accept -1 length for NUL-terminated cstrings.