On Thu, Feb 29, 2024 at 10:55:20PM -0500, Corey Huinker wrote: >> That’s certainly a fair point and my initial reaction (which could >> certainly be wrong) is that it’s unlikely to be an issue- but also, if you >> feel you could make it work with an array and passing all the attribute >> info in with one call, which I suspect would be possible but just a bit >> more complex to build, then sure, go for it. If it ends up being overly >> unwieldy then perhaps the per-attribute call would be better and we could >> perhaps acquire the lock before the function calls..? Doing a check to see >> if we have already locked it would be cheaper than trying to acquire a new >> lock, I’m fairly sure. > > Well the do_analyze() code was already ok with acquiring the lock once for > non-inherited stats and again for inherited stats, so the locks were > already not the end of the world. However, that's at most a 2x of the > locking required, and this would natts * x, quite a bit more. Having the > procedures check for a pre-existing lock seems like a good compromise.
I think this is a reasonable starting point. If the benchmarks show that the locking is a problem, we can reevaluate, but otherwise IMHO we should try to keep it as simple/flexible as possible.
Yeah, this was my general feeling as well. If it does become an issue, it certainly seems like we would have ways to improve it in the future. Even with this locking it is surely going to be better than having to re-analyze the entire database which is where we are at now.