Re: Materialized views WIP patch

Marko Tiikkaja wrote:
> On 15/11/2012 03:28, Kevin Grittner wrote:

> I have been testing the patch a bit

Thanks!

> and I'm slightly disappointed by the fact that it still doesn't
> solve this problem (and I apologize if I have missed discussion
> about this in the docs or in this thread):
>
> <assume "foo" is a non-empty materialized view>
>
> T1: BEGIN;
> T1: LOAD MATERIALIZED VIEW foo;
>
> T2: SELECT * FROM foo;
>
> T1: COMMIT;
>
> <T2 sees an empty table>

As far as I know you are the first to notice this behavior. Thanks
for pointing it out.

I agree with Robert that we have to be careful about scope creep,
but this one looks to me like it should be considered a bug. IMO,
LOAD for a populated MV should move it from one state which
reflects the captured state of a previous point in time to a
captured state which is later, with no invalid or inconsistent
states visible in between.

I will take a look at the issue; I don't know whether it's
something small I can address in this CF or whether it will need to
be in the next CF, but I will fix it.

>> 19. LMV doesn't show a row count. It wouldn't be hard to add, it
>>     just seemed a little out of place to do that, when CLUSTER,
>>     etc., don't.
>
> This sounds like a useful feature, but your point about CLUSTER
> and friends still stands.

Other possible arguments against providing a count are:

(1) For a populated MV, the LOAD might be replacing the contents
with fewer rows than were there before.

(2) Once we have incremental updates of the MV, this count is only
one of the ways to update the view -- and the others won't show
counts. Showing it here might be considered inconsistent.

I don't feel strongly about it, and I don't think it's a big change
either way; just explaining what got me off the fence when I had to
pick one behavior or the other to post the WIP patch.

> I'll get back when I manage to get a better grasp of the code.

Thanks.

Keep in mind that the current behavior of behaving like a regular
view when the contents are invalid is not what I had in mind, that
was an accidental effect of commenting out the body of the
ExecCheckRelationsValid() function right before posting the patch
because I noticed a regression. When I noticed current behavior, it
struck me that someone might prefer it to the intended behavior of
showing an error like this:

 ereport(ERROR,
   (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    errmsg("materialized view \"%s\" has not been populated",
           get_rel_name(rte->relid)),
    errhint("Use the LOAD MATERIALIZED VIEW command.")));

I mention it in case someone wants to argue for silently behaving
as a regular view when the MV is not populated.

-Kevin

Hi Kevin,

On Mon, 26 Nov 2012 22:24:33 +0100, Kevin Grittner <kgrittn(at)mail(dot)com>
wrote:
> Marko Tiikkaja wrote:
>> <T2 sees an empty table>
>
> As far as I know you are the first to notice this behavior. Thanks
> for pointing it out.
>
> I will take a look at the issue; I don't know whether it's
> something small I can address in this CF or whether it will need to
> be in the next CF, but I will fix it.

Any news on this front?

>> I'll get back when I manage to get a better grasp of the code.

The code looks relatively straightforward and good to my eyes. It passes
my testing and looks to be changing all the necessary parts of the code.

> Keep in mind that the current behavior of behaving like a regular
> view when the contents are invalid is not what I had in mind, that
> was an accidental effect of commenting out the body of the
> ExecCheckRelationsValid() function right before posting the patch
> because I noticed a regression. When I noticed current behavior, it
> struck me that someone might prefer it to the intended behavior of
> showing an error like this:
>
> ereport(ERROR,
> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> errmsg("materialized view \"%s\" has not been populated",
> get_rel_name(rte->relid)),
> errhint("Use the LOAD MATERIALIZED VIEW command.")));
>
> I mention it in case someone wants to argue for silently behaving
> as a regular view when the MV is not populated.

FWIW, I'd prefer an error in this case, but I don't feel strongly about it.

Regards,
Marko Tiikkaja