Re: Incremental View Maintenance, take 2

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Tatsuo Ishii <ishii(at)sraoss(dot)co(dot)jp>
Cc: nagata(at)sraoss(dot)co(dot)jp, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Incremental View Maintenance, take 2
Date: 2023-09-04 08:48:02
Message-ID: CACJufxFjankFQDNppOfqCTpY=zW4Q0+2WCmKjT95kggiT978Lw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Sep 2, 2023 at 7:46 PM Tatsuo Ishii <ishii(at)sraoss(dot)co(dot)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?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-09-04 09:50:04 Re: persist logical slots to disk during shutdown checkpoint
Previous Message Amit Kapila 2023-09-04 08:42:58 Re: pg_upgrade and logical replication