On 1/25/13 1:00 AM, Kevin Grittner wrote: > New patch rebased, fixes issues raised by Thom Brown, and addresses > some of your points.
This patch doesn't apply anymore, so I just took a superficial look. I think the intended functionality and the interfaces look pretty good. Documentation looks complete, tests are there. I have a couple of notes: * What you call WITH [NO] DATA, Oracle calls BUILD IMMEDIATE/DEFERRED. It might be better to use that as well then. * You use fields named relkind in the parse nodes, but they don't actually contain relkind values, which is confusing. I'd just name the field is_matview or something. * More generally, I wouldn't be so fond of combining the parse handling of CREATE TABLE AS and CREATE MATERIALIZED VIEW. They are similar, but then again so are a lot of other things. * Some of the terminology is inconsistent. A materialized view is sometimes called valid, populated, or built, with approximately the same meaning. Personally, I would settle on "built", as per above, but it should be one term only. * I find the name of the relisvalid column a bit confusing. Especially because it only applies to materialized views, and there is already a meaning of "valid" for indexes. (Recall that indexes are also stored in pg_class, but they are concerned about indisvalid.) I would name it something like relmvbuilt. Btw., half of the patch seems to consist of updating places referring to relkind. Is something wrong with the meaning of relkind that this sort of thing is required? Maybe these places should be operating in terms of features, not accessing relkind directly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers