| From: | Alexander Lakhin <exclusion(at)gmail(dot)com> |
|---|---|
| To: | Michael Paquier <michael(at)paquier(dot)xyz>, jian he <jian(dot)universality(at)gmail(dot)com> |
| Cc: | Ajin Cherian <itsajin(at)gmail(dot)com>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, 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-12-06 07:00:01 |
| Message-ID: | b95c51d5-6089-f8ee-dcfc-94b620b21ff3@gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Michael,
05.12.2023 02:45, Michael Paquier wrote:
> Popping a snapshot at this stage when there are no indexes has been a
> decision taken by the original commit in 5dc92b844e68 because we had
> no need for it yet, but we may do now depending on the function
> triggered. I have been looking at the whole stack and something like
> the attached to make a pop conditional seems to be sufficient to
> satisfy all the cases I've been able to come up with, including the
> one reported here.
>
> Alexander, do you see any hole in that? Perhaps the snapshot push
> should be more aggressive at the end of ReindexRelationConcurrently()
> as well (in the last transaction when rebuilds happen)?
Thank you for the fix proposed!
I agree with it. I had worried a bit about ReindexRelationConcurrently()
becoming twofold for callers (it can leave the snapshot or pop it), but I
couldn't find a way to hide this twofoldness inside without adding more
complexity. On the other hand, ReindexRelationConcurrently() now satisfies
EnsurePortalSnapshotExists() in all cases.
Best regards,
Alexander
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Junwang Zhao | 2023-12-06 07:11:34 | Re: Make COPY format extendable: Extract COPY TO format implementations |
| Previous Message | Richard Guo | 2023-12-06 06:51:27 | Re: Oversight in reparameterize_path_by_child leading to executor crash |