Re: Materialized views WIP patch

Noah Misch <noah(at)leadboat(dot)com> wrote:

> Like Kevin, I want a way to distinguish unpopulated MVs from MVs that
> genuinely yielded the empty set at last refresh.  I agree that there's no
> particular need to store that fact in pg_class, and I would much prefer only
> storing it one way in any case.  A user-visible disadvantage of the current
> implementation is that relisvalid remains stale until something opens the rel.
> That's fine for the system itself, but it can deceive user-initiated catalog
> queries.  Imagine a check_postgres action that looks for invalid MVs to
> complain about.  It couldn't just scan pg_class; it would need to first do
> something that opens every MV.
>
> I suggest the following:
>
> 1. Let an invalid MV have a zero-length heap.  Distinguish a valid, empty MV
>    by giving it a page with no tuples.  This entails VACUUM[1] not truncating
>    MVs below one page and the refresh operation, where necessary, extending
>    the relation from zero pages to one.
> 2. Remove pg_class.relisvalid.
> 3. Add a bool field to RelationData.  The word "valid" is used in that context
>    to refer to the validity of the structure itself, so perhaps call the new
>    field rd_scannable.  RelationIsFlaggedAsValid() can become a macro;
>    consider changing its name for consistency with the field name.
> 4. During relcache build, set the field to "RelationGetNumberBlocks(rel) !=> 0"
>    for MVs, fixed "true" for everyone else.  Any operation that changes the
>    field must, and probably would anyway, instigate a relcache invalidation.
> 5. Expose a database function, say pg_relation_scannable(), for querying the
>    current state of a relation.  This supports user-level monitoring.
>
> Does that seem reasonable?  One semantic difference to keep in mind is that
> unlogged MVs will be considered invalid on the standby while valid on the
> master.  That's essentially an accurate report, so I won't mind it.

Changed to work pretty much as you suggested.

> I'm going to follow this with a review covering a broader range of topics.

Those issues addressed, too.  That includes the most egregious doc
problems you pointed out, but there still needs to be a thorough
review, and I expect to find a few more doc cleanup issues.

There was one minor syntax issue not addressed by Noah, nor much
discussed in general that I didn't want to just unilaterally
choose; but given that nobody seems to care that much I will put
forward a proposal and do it that way tomorrow if nobody objects.
Before this patch tables were the only things subject to
truncation, but now materialized views can also be truncated.  So
far we have been treating TABLE as a noise word in the truncate
command.  I assume we still want to allow tables to be truncated
with or without the word.  The question is what to do about
materialized views, and wheter both can be specified on a single
TRUNCATE statement.  I propose that we allow TABLE or MATERIALIZED
VIEW to be specified, or that part of the statement to be left out.
I propose that either type of object be allowed unless one or the
other is specified and the object to be truncated is not of that
kind.  So you could mix both kinds on one statement, so long as you
didn't specify either kind.

There is one odd aspect to pg_dump, but I think the way it is
behaving is the best way to handle it, although I invite other
opinions.  If you load from pg_dump output, it will try to
populated materialized views which were populated on dump, and
leave the ones which were not scannable because the contents had
not been generated in an empty and unscannable state on restore.
That much seems pretty obvious.  Where it gets  a little tricky is
if mva is generated with data, and mvb is generated based on mva.
Then mva is truncated.  Then you dump.  mvb was populated at the
time of the dump, but its contents can't be regenerated on restore
because mva is not scannable.  As the patch currently stands, you
get an error on the attempt to REFRESH mvb.  I think that's a good
thing, but I'm open to arguments to the contrary.

I don't have any handling in pg_dump for circular references among
materialized views, because I couldn't see how that could happen.
I'm not 100% sure that isn't just a failure of imagination on my
part, though.

The only other comment I know of that hasn't been addressed is
Simon's suggestion that I put in syntax for features which we might
implement in future releases.  I don't want to do that without the
usual community design and bike-shedding process, so syntax is only
implemented for implemented features.

I'm still waiting for final word (and a small patch?) from KaiGai
Kohei for the sepgsql part.

This patch does require an initdb because of a new function.

Unless something else comes up in review or I get feedback to the
contrary I plan to deal with the above-mentioned issues and commit
this within a week or two.

Thanks to Marko Tiikkaja, Robert Haas, Thom Brown, Simon Riggs,
KaiGai Kohei, and Noah Misch for the reviews and suggestions so
far, thanks to Robert for the initial cut at the docs, and big
thanks to Noah for helping me track down an elusive bug.

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