| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
| Cc: | jian he <jian(dot)universality(at)gmail(dot)com>, Garrett Thornburg <film42(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: PATCH: Add REINDEX tag to event triggers |
| Date: | 2023-10-27 07:15:19 |
| Message-ID: | ZTtjh6UtGw9zyvX6@paquier.xyz |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2023-10-27 07:22:54 | Re: Requiring recovery.signal or standby.signal when recovering with a backup_label |
| Previous Message | Étienne BERSAC | 2023-10-27 07:14:38 | Re: run pgindent on a regular basis / scripted manner |