Re: PATCH: Add REINDEX tag to event triggers

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

In response to

Responses

Browse pgsql-hackers by date

  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