Re: PATCH: Add REINDEX tag to event triggers

Garrett Thornburg <film42@gmail.com>

From: Garrett Thornburg <film42@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: Michael Paquier <michael@paquier.xyz>, jian he <jian.universality@gmail.com>
Date: 2023-07-27T04:43:01Z
Lists: pgsql-hackers

Attachments

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
>

Commits

  1. Fix assertion failure with REINDEX and event triggers

  2. Add support for REINDEX in event triggers

  3. Doc: Improve documentation for creating custom scan paths.

  4. Add const decorations