Re: Remaining beta blockers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> [ shrug... ]  You and Kevin essentially repeated your claims that
> the current implementation is OK; nobody else weighed in.

Many people weighed in on the need to differentiate between an
empty matview and one which has not been populated.  Many have also
weighed in on the benefits of unlogged matviews.

> I see absolutely no reason to change my opinion on this.  Keeping
> relisscannable state in the form of is-the-file-of-nonzero-size
> is something we WILL regret

... or change in a subsequent major release.  I see no reason why
using this technique now make it harder to do something else later.
Could you elaborate on the technical challenges you see to doing
so?

> Pollyanna-ish refusal to believe that is not an adequate reason
> for painting ourselves into a corner, especially not for a
> second-order feature like unlogged matviews.

I would guess that about half the use-cases for materialized views
will stay with tables in spite of the added hassle, if they have to
degrade performance by adding logging where they currently have
none.

> The way forward to unlogged matviews that behave the way Kevin
> wants is to improve crash recovery so that we can update catalog
> state after completing recovery, which is something there are
> other reasons to want anyway.

That would certainly be for the best.

> But it's far too late to do that in this cycle.

Yes it is.

> In the meantime I remain convinced that we're better off dropping
> the feature until we have an implementation that's not going to
> bite us in the rear in the future.

I haven't caught up with you on how it will do that.

> I also note that there are acknowledged-even-by-you bugs in the
> current implementation, which no patch has emerged for, so
> *something's* got to be done.  I'm done waiting for something to
> happen, and am going to go fix it in the way I think best.

Are you referring to the case where if you refresh a matview with
over 8GB and it winds up empty, that vacuum can make it look
invalid until the next REFRESH?  This is one of many ways that
could be fixed.

diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 8a1ffcf..b950b16 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -230,7 +230,13 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
      *
      * Don't even think about it unless we have a shot at releasing a goodly
      * number of pages.  Otherwise, the time taken isn't worth it.
+     *
+     * Leave a populated materialized view with at least one page.
      */
+    if (onerel->rd_rel->relkind == RELKIND_MATVIEW &&
+        vacrelstats->nonempty_pages == 0)
+        vacrelstats->nonempty_pages = 1;
+
     possibly_freeable = vacrelstats->rel_pages - vacrelstats->nonempty_pages;
     if (possibly_freeable > 0 &&
         (possibly_freeable >= REL_TRUNCATE_MINIMUM ||

The hysteria and FUD about using the currently-supported technique
for unlogged tables to implement unlogged matviews are very
discouraging.  And the notion that we would release a matview
feature which allowed false results (in the form of treating a
never-populated matview as a legal empty matview) is truly scary to
me.  If we can't tell the difference between those two things, I
don't think we should be releasing the feature.

--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company