Re: PATCH: Add REINDEX tag to event triggers - Mailing list pgsql-hackers

From Garrett Thornburg
Subject Re: PATCH: Add REINDEX tag to event triggers
Date
Msg-id CAEEqfk6_30n2grUMizuDK+cKs_Ncz5LJTqBnoRuZieT-=1wGPA@mail.gmail.com
Whole thread Raw
In response to PATCH: Add REINDEX tag to event triggers  (Garrett Thornburg <film42@gmail.com>)
Responses Re: PATCH: Add REINDEX tag to event triggers
List pgsql-hackers
I added a v2 patch for adding REINDEX to event triggers. The following has changed:

1. I fixed the docs to note that ddl_command_start is supported for REINDEX. Thanks, Michael!
2. I added Jian He's excellent partition table test and updated the expectations to include that regression test.

What is still up for debate:

Michael pointed out that REINDEX probably isn't DDL because the docs only specify commands in the standard like CREATE, ALTER, etc. It might be worth creating a new event to trigger on (similar to what was done for table_rewrite) to make a REINDEX trigger fit in a little nicer. Let me know what you think here. Until then, I left the code as `lev = LOGSTMT_DDL` for `T_ReindexStmt`.

Thanks,
Garrett

On Thu, Jul 20, 2023 at 10:47 PM Garrett Thornburg <film42@gmail.com> wrote:
Added my v1 patch to add REINDEX to event triggers.

I originally built this against pg15 but rebased to master for the patch to hopefully make it easier for maintainers to merge. The rebase was automatic so it should be easy to include into any recent version. I'd love for this to land in pg16 if possible.

I added regression tests and they are passing. I also formatted the code using the project tools. Docs are included too.

A few notes:
1. I tried to not touch the function parameters too much and opted to create a convenience function that makes it easy to attach index or table OIDs to the current event trigger list.
2. I made sure both concurrent and regular reindex of index, table, database work as expected.
3. The ddl end command will make available all indexes that were modified by the reindex command. This is a large list when you run "reindex database" but works really well. I debated returning records of the table or DB that were reindexed but that doesn't really make sense. Returning each index fits my use case of building an audit record around the index lifecycle (hence the motivation for the patch).

Thanks for your time,

Garrett Thornburg
Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: WAL Insertion Lock Improvements
Next
From: Amit Kapila
Date:
Subject: Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication