public inbox for pgsql-hackers@postgresql.org  

help / color / mirror / Atom feed

From: jian he <jian.universality@gmail.com>
To: Tatsuo Ishii <ishii@sraoss.co.jp>
Cc: nagata@sraoss.co.jp
Cc: pgsql-hackers@postgresql.org
Subject: Re: Incremental View Maintenance, take 2
Date: Mon, 4 Sep 2023 16:48:02 +0800
Message-ID: <CACJufxFjankFQDNppOfqCTpY=zW4Q0+2WCmKjT95kggiT978Lw@mail.gmail.com> (raw)
In-Reply-To: <20230902.204634.955758704959569058.t-ishii@sranhm.sra.co.jp>
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>

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?

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: jian.universality@gmail.com, ishii@sraoss.co.jp, nagata@sraoss.co.jp
  Subject: Re: Incremental View Maintenance, take 2
  In-Reply-To: <CACJufxFjankFQDNppOfqCTpY=zW4Q0+2WCmKjT95kggiT978Lw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox