MIME-Version: 1.0
References: <20230828115252.c1b018605b9a0756a30c3382@sraoss.co.jp>
 <20230828160530.adde1e20f257d7d345989163@sraoss.co.jp> <CACJufxEoCCJE1vntJp1SWjen8vBUa3vZLgL=swPwar4zim976g@mail.gmail.com>
 <20230902.204634.955758704959569058.t-ishii@sranhm.sra.co.jp>
In-Reply-To: <20230902.204634.955758704959569058.t-ishii@sranhm.sra.co.jp>
From: jian he <jian.universality@gmail.com>
Date: Mon, 4 Sep 2023 16:48:02 +0800
Message-ID: <CACJufxFjankFQDNppOfqCTpY=zW4Q0+2WCmKjT95kggiT978Lw@mail.gmail.com>
Subject: Re: Incremental View Maintenance, take 2
To: Tatsuo Ishii <ishii@sraoss.co.jp>
Cc: nagata@sraoss.co.jp, pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CACJufxFjankFQDNppOfqCTpY%3DzW4Q0%2B2WCmKjT95kggiT978Lw%40mail.gmail.com>
Precedence: bulk

On Sat, Sep 2, 2023 at 7:46=E2=80=AFPM Tatsuo Ishii <ishii@sraoss.co.jp> wr=
ote:
>
> > attached is my refactor. there is some whitespace errors in the
> > patches, you need use
> > git apply --reject --whitespace=3Dfix
> > 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 >=3D entry->after_trig_count);
    if (entry->before_trig_count !=3D 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?