Re: PATCH: Add REINDEX tag to event triggers

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Jim Jones <jim.jones@uni-muenster.de>
Cc: jian he <jian.universality@gmail.com>, Garrett Thornburg <film42@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2023-10-27T07:15:19Z
Lists: pgsql-hackers
On Mon, Sep 04, 2023 at 08:00:52PM +0200, Jim Jones wrote:
> LGTM. It applies and builds cleanly, all tests pass and documentation also
> builds ok. The CFbot seems also much happier now :)

+	/*
+	 * Open and lock the relation.  ShareLock is sufficient since we only need
+	 * to prevent schema and data changes in it.  The lock level used here
+	 * should match catalog's reindex_relation().
+	 */
+	rel = try_table_open(relid, ShareLock);

I was eyeing at 0003, and this strikes me as incorrect.  Sure, this
matches what reindex_relation() does, but you've missed that
CONCURRENTLY takes a lighter ShareUpdateExclusiveLock, and ShareLock
conflicts with it.  See:
https://www.postgresql.org/docs/devel/explicit-locking.html

So, doesn't this disrupt a concurrent REINDEX?
--
Michael

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