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
- v2-0001-Add-REINDEX-tag-to-event-triggers.patch (application/octet-stream) patch v2-0001
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
-
Fix assertion failure with REINDEX and event triggers
- c426f7c2b36a 17.0 landed
-
Add support for REINDEX in event triggers
- f21848de2013 17.0 landed
-
Doc: Improve documentation for creating custom scan paths.
- 7ef5f5fb3240 17.0 cited
-
Add const decorations
- 11af63fb48d2 17.0 cited