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
