On Sat, Dec 24, 2016 at 1:18 AM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
> Alvaro Herrera wrote: > > > 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. Thanks, Pavan -- Pavan Deolasee http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services