Re: Avoid calling SetMatViewPopulatedState if possible

Hi,

> Attached is an updated patch. I also adjusted the nearby comment and
> expanded the commit message to explain the unnecessary pg_class update
> and resulting dead tuple generation.

+1 on the change. I have the same thing in the REFRESH ... WHERE patch
[1], so I'd like to see it go in on its own.

One thing worth adding to the justification: the dead tuple isn't the
only cost. The CatalogTupleUpdate() also sends an SI message, so every
call invalidates the relcache entry for the MV in every backend,
including the caller's own, and that discards cached plans referencing
it. On an MV that gets refreshed often and is already populated, the
repeated invalidation costs more than the dead tuple does.

A partial refresh hits this on every call, which is how I ran into it.
That patch caches the prepared refresh plans per MV to get the per-call
cost down, and until I added the check the invalidation was marking them
invalid on every refresh, so the cache wasn't buying anything.

I had put the check inside SetMatViewPopulatedState() rather than at the
call site, but there's no functional difference between the two. Both
call sites are covered either way, so that's just a style preference on
my part. I'll rebase onto whatever form goes in.

Thanks,
Adam

[1]
https://www.postgresql.org/message-id/flat/CAMjNa7eFzTQ5%3DoZMQiB2bMkez5KP4A77JC7SRjeVEkOrh7cUHw%40mail.gmail.com