Re: Incremental View Maintenance, take 2

On Sat, Sep 2, 2023 at 7:46 PM Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
>
> > attached is my refactor. there is some whitespace errors in the
> > patches, you need use
> > git apply --reject --whitespace=fix
> > basedon_v29_matview_c_refactor_update_set_clause.patch
> >
> > Also you patch cannot use git apply, i finally found out bulk apply
>
> I have no problem with applying Yugo's v29 patches using git apply, no
> white space errors.
>

thanks. I downloaded the patches from the postgres website, then the
problem was solved.

other ideas based on v29.

src/include/utils/rel.h
680: #define RelationIsIVM(relation) ((relation)->rd_rel->relisivm)
I guess it would be better to add some comments to address the usage.
Since all peer macros all have some comments.

pg_class change, I guess we need bump CATALOG_VERSION_NO?

small issue. makeIvmAggColumn and calc_delta need to add an empty
return statement?

style issue. in gram.y, "incremental" upper case?
+               CREATE OptNoLog incremental MATERIALIZED VIEW
create_mv_target AS SelectStmt opt_with_data

I don't know how pgident works, do you need to add some keywords to
src/tools/pgindent/typedefs.list to make indentation work?

in
    /* If this is not the last AFTER trigger call, immediately exit. */
    Assert (entry->before_trig_count >= entry->after_trig_count);
    if (entry->before_trig_count != entry->after_trig_count)
        return PointerGetDatum(NULL);

before returning NULL, do you also need clean_up_IVM_hash_entry? (I
don't know when this case will happen)

in
        /* Replace the modified table with the new delta table and
calculate the new view delta*/
        replace_rte_with_delta(rte, table, true, queryEnv);
        refresh_matview_datafill(dest_new, query, queryEnv, tupdesc_new, "");

replace_rte_with_delta does not change the argument: table, argument:
queryEnv. refresh_matview_datafill just uses the partial argument of
the function calc_delta. So I guess, I am confused by the usage of
replace_rte_with_delta. also I think it should return void, since you
just modify the input argument. Here refresh_matview_datafill is just
persisting new delta content to dest_new?