On Thu, Oct 31, 2024 at 08:45:15AM +0900, Michael Paquier wrote: > On Wed, Oct 30, 2024 at 03:54:32PM -0500, Nathan Bossart wrote: >> I'll manage. 0001 was a doozy to back-patch, and this obviously isn't a >> terribly pressing issue, so I plan to wait until after the November minor >> release to apply this. > > Okay by me.
I took another look at 0001, and I think it still needs a bit of work. Specifically, IIUC a few of these code paths are actually fine since they do not ever touch a varlena column. For example, clear_subscription_skip_lsn() only updates subskiplsn, so unless I am missing a corner case, there is no risk of trying to access a TOAST table without a snapshot. Allowing such cases would involve adjusting the new assertions in 0003 to check for only the varlena columns during inserts/updates, which is more complicated but seems doable. Or we could just enforce that you have an active snapshot whenever you modify a catalog with a TOAST table. That's simpler, but it requires extra work in some paths (and probably comments to point out that we're only pushing an active snapshot to satisfy an assertion). >> I'm having second thoughts on 0002, so I'll >> probably leave that one uncommitted, but IMHO we definitely need 0003 to >> prevent this issue from reoccurring. > > I liked your idea behind 0002, FWIW. We do that for a lot of fields > already in a Relation. Alright, then I'll plan on proceeding with it. -- nathan
