From: | Garrett Thornburg <film42(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, jian he <jian(dot)universality(at)gmail(dot)com> |
Subject: | Re: PATCH: Add REINDEX tag to event triggers |
Date: | 2023-07-27 04:43:01 |
Message-ID: | CAEEqfk6_30n2grUMizuDK+cKs_Ncz5LJTqBnoRuZieT-=1wGPA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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(at)gmail(dot)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 | Content-Type | Size |
---|---|---|
v2-0001-Add-REINDEX-tag-to-event-triggers.patch | application/octet-stream | 21.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2023-07-27 05:02:46 | Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication |
Previous Message | Michael Paquier | 2023-07-27 04:36:43 | Re: WAL Insertion Lock Improvements |