public inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Kirill Reshke <reshkekirill@gmail.com>
To: Yugo NAGATA <nagata@sraoss.co.jp>
Cc: Peter Smith <smithpb2250@gmail.com>
Cc: jian he <jian.universality@gmail.com>
Cc: Tatsuo Ishii <ishii@sraoss.co.jp>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Incremental View Maintenance, take 2
Date: Thu, 8 Aug 2024 14:37:54 +0500
Message-ID: <CALdSSPgdkrqZjA7Wb11essc6_=ZgSxjs+frUE3w=bJDu_0mf6A@mail.gmail.com> (raw)
In-Reply-To: <20240711132357.fe3f78c184cfa99159208178@sranhm.sraoss.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>
<CACJufxFjankFQDNppOfqCTpY=zW4Q0+2WCmKjT95kggiT978Lw@mail.gmail.com>
<CAHut+PsDpBTxZ7bLhko7_E-C7khMhoNJcriNQ_p_gWjADn01vg@mail.gmail.com>
<20240123162327.c2803162619dd7634cca0b6c@sraoss.co.jp>
<20240304115846.2275fb44fd904e8789d43590@sraoss.co.jp>
<20240329234700.73ff2e28c9248d29f8fa6a66@sraoss.co.jp>
<20240331225931.712683cecb26862b73b2b822@sraoss.co.jp>
<20240702170311.1ddb417759a48ff12c555b92@sranhm.sraoss.co.jp.sranhm>
<20240711132357.fe3f78c184cfa99159208178@sranhm.sraoss.co.jp>
I am really sorry for splitting my review comments into multiple
emails. I'll try to do a better review in a future, all-in-one.
On Thu, 11 Jul 2024 at 09:24, Yugo NAGATA <nagata@sraoss.co.jp> wrote:
>
> On Tue, 2 Jul 2024 17:03:11 +0900
> Yugo NAGATA <nagata@sraoss.co.jp> wrote:
>
> > On Sun, 31 Mar 2024 22:59:31 +0900
> > Yugo NAGATA <nagata@sraoss.co.jp> wrote:
> > > >
> > > > Also, I added a comment on RelationIsIVM() macro persuggestion from jian he.
> > > > In addition, I fixed a failure reported from cfbot on FreeBSD build caused by;
> > > >
> > > > WARNING: outfuncs/readfuncs failed to produce an equal rewritten parse tree
> > > >
> > > > This warning was raised since I missed to modify outfuncs.c for a new field.
> > >
> > > I found cfbot on FreeBSD still reported a failure due to
> > > ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS because the regression test used
> > > wrong role names. Attached is a fixed version, v32.
> >
> > Attached is a rebased version, v33.
>
> I updated the patch to bump up the version numbers in psql and pg_dump codes
> from 17 to 18.
>
> Regards,
> Yugo Nagata
>
> >
> > Regards,
> > Yugo Nagata
> >
> >
> > --
> > Yugo NAGATA <nagata@sraoss.co.jp>
>
>
> --
> Yugo NAGATA <nagata@sraoss.co.jp>
1) Provided patches do not set process title correctly:
```
reshke 2602433 18.7 0.1 203012 39760 ? Rs 20:41 1:58
postgres: reshke ivm [local] CREATE MATERIALIZED VIEW
```
2) We allow to REFRESH IMMV. Why? IMMV should be always up to date.
Well, I can see that this utility command may be useful in case of
corruption of some base relation/view itself, so there will be a need
to rebuild the whole from scratch.
But we already have VACUUM FULL for this, aren't we?
3) Triggers created for IMMV are not listed via \dS [tablename]
4) apply_old_delta_with_count executes non-trivial SQL statements for
IMMV. It would be really helpful to see this in EXPLAIN ANALYZE.
5)
> + "DELETE FROM %s WHERE ctid IN ("
> + "SELECT tid FROM (SELECT pg_catalog.row_number() over (partition by %s) AS \"__ivm_row_number__\","
> + "mv.ctid AS tid,"
> + "diff.\"__ivm_count__\""
> + "FROM %s AS mv, %s AS diff "
> + "WHERE %s) v "
> + "WHERE v.\"__ivm_row_number__\" OPERATOR(pg_catalog.<=) v.\"__ivm_count__\")",
> + matviewname,
> + keysbuf.data,
> + matviewname, deltaname_old,
> + match_cond);
`SELECT pg_catalog.row_number()` is too generic to my taste. Maybe
pg_catalog.immv_row_number() / pg_catalog.get_immv_row_number() ?
6)
> +static void
> +apply_new_delta(const char *matviewname, const char *deltaname_new,
> + StringInfo target_list)
> +{
> + StringInfoData querybuf;
>+
> + /* Search for matching tuples from the view and update or delete if found. */
Is this comment correct? we only insert tuples here?
7)
During patch development, one should pick OIDs from range 8000-9999
> +# IVM
> +{ oid => '786', descr => 'ivm trigger (before)',
> + proname => 'IVM_immediate_before', provolatile => 'v', prorettype => 'trigger',
> + proargtypes => '', prosrc => 'IVM_immediate_before' },
> +{ oid => '787', descr => 'ivm trigger (after)',
> + proname => 'IVM_immediate_maintenance', provolatile => 'v', prorettype => 'trigger',
> + proargtypes => '', prosrc => 'IVM_immediate_maintenance' },
> +{ oid => '788', descr => 'ivm filetring ',
> + proname => 'ivm_visible_in_prestate', provolatile => 's', prorettype => 'bool',
> + proargtypes => 'oid tid oid', prosrc => 'ivm_visible_in_prestate' },
> ]
--
Best regards,
Kirill Reshke
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: reshkekirill@gmail.com, nagata@sraoss.co.jp, smithpb2250@gmail.com, jian.universality@gmail.com, ishii@sraoss.co.jp
Subject: Re: Incremental View Maintenance, take 2
In-Reply-To: <CALdSSPgdkrqZjA7Wb11essc6_=ZgSxjs+frUE3w=bJDu_0mf6A@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