> With your WARM and my indirect indexes, plus the additions for for-key > locks, plus identity columns, there is no longer a real expectation that > we can exit early from the function. In your patch, as well as mine, > there is a semblance of optimization that tries to avoid computing the > updated_attrs output bitmapset if the pointer is not passed in, but it's > effectively pointless because the only interesting use case is from > ExecUpdate() which always activates the feature. Can we just agree to > drop that?
Yes, I agree. As you noted below, the only case that may be affected is simple_heap_update() which does a lot more and hence this function will be least of the worries.
I think the only case that gets worse is the path that does simple_heap_update, which is used for DDL. I would be very surprised if a change there is noticeable, when compared to the rest of the stuff that goes on for DDL commands.
Now, after saying that, I think that a table with a very large number of columns is going to be affected by this. But we don't really need to compute the output bits for every single column -- we only care about those that are covered by some index. So we should pass an input bitmapset comprising all such columns, and the output bitmapset only considers those columns, and ignores columns not indexed. My patch for indirect indexes already does something similar (though it passes a bitmapset of columns indexed by indirect indexes only, so it needs a tweak there.)
Yes, that looks like a good compromise. This would require us to compare only those columns that any caller of the function might be interested in.