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