Date: Tue, 30 Jul 2024 14:24:20 +0900
From: Yugo NAGATA <nagata@sraoss.co.jp>
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, jian he
 <jian.universality@gmail.com>, Tatsuo Ishii <ishii@sraoss.co.jp>,
 pgsql-hackers@postgresql.org
Subject: Re: Incremental View Maintenance, take 2
Message-Id: <20240730142420.34a9ad7c249aecde88cd45fb@sraoss.co.jp>
In-Reply-To: <CALdSSPip9ruUoQMmsD_hQ0xY72qB=_jB-ayHeUWUH-dd0MB60A@mail.gmail.com>
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>
	<CALdSSPhj1H1NS7QiYkSQNCksPCwjtLcyt3==evgkBX1SrKyVdQ@mail.gmail.com>
	<CALdSSPip9ruUoQMmsD_hQ0xY72qB=_jB-ayHeUWUH-dd0MB60A@mail.gmail.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?GHBPLVBITN8da4JeM4O+7NZURnw5oFR75K/0v2538tKgzwr30eWyzr3ztGSw?=
 =?us-ascii?Q?YYss7zYoG2SE4KdSedlqAGNgstrgFBCuRcVyLe7wYWbFmme+AtfRgLXQvFGy?=
 =?us-ascii?Q?g3vW9zi8qEryyHqP71smT/Vgi1Wy3aFH1hYFBuBBGN0HFaucbolCJ2A26TQf?=
 =?us-ascii?Q?LbDPpntG/zmOOzkM69z4FaZOIAxX4ExvPwpRfREIJZWWYIFUIfsLeZ8cv8I+?=
 =?us-ascii?Q?aF8O89kkG9NAw9rXDTRt+OTLITMjUYTGdPZ6ZsBK/z3y7+7dWq69vEZzzy2M?=
 =?us-ascii?Q?AiZ8+kjlT32Y9IwXmdlFB89N9+VIbWEDRgp01KJXo+f9RC86rDLVYvB5oyhg?=
 =?us-ascii?Q?4C0IU+nq2Tt7bOl5SrBh3ZM5TeEBqYZOfPNV4s18mOMS4u0LBdLeHz6H/aoT?=
 =?us-ascii?Q?3xD6kJwI6dBXdQJKyohuyQOkOSY7HUHacJYTeIdYPceN1/QX9C7KQXnNCinx?=
 =?us-ascii?Q?JG5GUYvDTSWdd+6yzJJjUIV6Q0qxLacxKqfwiRLBXHHt91Sh9QwB5uF0aFSb?=
 =?us-ascii?Q?Uxdf+kUsmvDsJv/TdRmDeR1O0dVW1zyRjcnwJZcfRDU4z63Y7KmMOJOo6hXW?=
 =?us-ascii?Q?1qlKqZ07W7xV4a9AsOtN/GollXx5XSowUCDVkDf5JrXO7UvIqErAUd3Aibla?=
 =?us-ascii?Q?dhyQ5bYXWzRLS7Q2ZwYR8FAVwdtBdDiA0jjegxHXumAmd4oVnbleSuelCqsB?=
 =?us-ascii?Q?JsGBv0rjr2iQqzFeyzmSO1kbRo9h/HIb9/PPJbpjA6tVAdJbsNv+fcSpeAxR?=
 =?us-ascii?Q?0QlweFSXWmNCTek5UmTKRTj0oXw7O4SC2YP3eKUYxUM3FNuVc9+S6nsDifnF?=
 =?us-ascii?Q?epvXLv8ytis9bVc9QndeRaAH+J6eKna09MQC+Z8521EqIrisJifuwPatTP+l?=
 =?us-ascii?Q?oAJH2etEHKB6yWI2BHjvjnOyfmogbjL12nhFQqdoJ+Ikic3rRJZS1Qfuse2/?=
 =?us-ascii?Q?073q5qRnQO0qvN+gqNKPPr/6wa3XFqD45YS5USEeRcFGOHfeuz1ovLe7CogL?=
 =?us-ascii?Q?ant3KOAHTftuJFe1qp6dCRvYztcZ4hF4D4Wu6YoZwEFR4gPplKqjKqrwUOCO?=
 =?us-ascii?Q?gzE5ObfxRlkVWa9W6f30D0nv1DH+1arnh4UPKWUkXPPb9IeZ58Hfio+PFb0v?=
 =?us-ascii?Q?8wpltRCgOLoPtsOOuKRXuObIrDDO/8Gn65sI9iNz6EOez/Vj/f8c7Cvv2X9i?=
 =?us-ascii?Q?XDYwebXi7VJoqVEWMUEgDm0g7z2ZCECXgft1pagaP877OVM3BmSIOCWEktKD?=
 =?us-ascii?Q?DId1f9GPgylSf+wWCiCpEQkchYuLHcaxZlCmuW9gA0xDKUrmNlS0jQiTUgii?=
 =?us-ascii?Q?QDm4ulUiKPVvSxOhJpZ0BKLOE1qPIg2bDRCyLCgp7MyO7SpbMcB8rfvMQFB+?=
 =?us-ascii?Q?raxoVW1KLt1PW93yGlOSik5Iaeu2/9iw1/s692rvNGRxxXLWWpTjOPCkf/dO?=
 =?us-ascii?Q?nQjX4jl4IOOx20AjR0ia1/4j9OgvvHHbzGAknc31HA3J6bTCe2BCwJWctn5J?=
 =?us-ascii?Q?M/evb/bWhwRxP6eE/2SR9w6B9H04kP2TD2ACvXcxWZBmktys6/WX4wNvIU3X?=
 =?us-ascii?Q?nkkr9fv8vZyZw8NJjfgE3bhJZRg/DfWpd4TFrjQlKA4m8Pc/nyZKk6QIn3nn?=
 =?us-ascii?Q?mSFQUMuhJePbDTSnjP3DPk8v403EcrTdls0wp3N9uw03?=
X-OriginatorOrg: sraoss.co.jp
X-MS-Exchange-CrossTenant-Network-Message-Id: 74d7cca7-4231-4f98-1779-08dcb057ee06
X-MS-Exchange-CrossTenant-AuthSource: TYBP286MB0111.JPNP286.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jul 2024 05:24:48.3736
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 75db6c32-352e-4063-ae79-cafb8623f6db
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: LCoFYcaCUKCkLb1eOllZntFfPG6fVegWBJudv3/QwxqDzf3w5WwV6gFM4s8ZRc0Zpo/ySGgdbSUh7WTEDh9oGw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYWP286MB2404
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: <https://www.postgresql.org/message-id/20240730142420.34a9ad7c249aecde88cd45fb%40sraoss.co.jp>
Precedence: bulk

Hi,

On Tue, 30 Jul 2024 03:32:19 +0500
Kirill Reshke <reshkekirill@gmail.com> wrote:

> On Sat, 27 Jul 2024 at 13:26, Kirill Reshke <reshkekirill@gmail.com> wrote:
> >
> > Hi!
> > Cloudberry DB (Greenplum fork) uses IMMV feature for AQUMV (auto query
> > use matview) feature, so i got interested in how it is implemented.

Thank you so much for a lot of comments!
I will respond to the comments soon.

> >
> > On Thu, 11 Jul 2024 at 09:24, Yugo NAGATA <nagata@sraoss.co.jp> wrote:
> > >
> > > I updated the patch to bump up the version numbers in psql and pg_dump codes
> > > from 17 to 18.
> >
> > Few suggestions:
> >
> > 1) `Add-relisivm-column-to-pg_class-system-catalog` commit message
> > should be fixed, there is "isimmv" in the last line.
> > 2) I dont get why `Add-Incremental-View-Maintenance-support.patch`
> > goes after 0005 & 0004. Shoulndt we first implement feature server
> > side, only when client (psql & pg_dump) side?
> > 3) Can we provide regression tests for each function separately? Test
> > for main feature in main patch, test for DISTINCT support in
> > v34-0007-Add-DISTINCT-support-for-IVM.patch etc? This way the patchset
> > will be easier to review, and can be committed separelety.
> > 4) v34-0006-Add-Incremental-View-Maintenance-support.patch no longer
> > applies due to 4b74ebf726d444ba820830cad986a1f92f724649. After
> > resolving issues manually, it does not compile, because
> > 4b74ebf726d444ba820830cad986a1f92f724649 also removes
> > save_userid/save_sec_context fields from ExecCreateTableAs.
> >
> > >   if (RelationIsIVM(matviewRel) && stmt->skipData)
> > Now this function accepts skipData param.
> >
> > 5) For DISTINCT support patch uses hidden __ivm* columns. Is this
> > design discussed anywhere? I wonder if this is a necessity (only
> > solution) or if there are alternatives.
> > 6)
> > What are the caveats of supporting some simple cases for aggregation
> > funcs like in example?
> > ```
> > regress=# CREATE INCREMENTAL MATERIALIZED VIEW mv_ivm_2 AS SELECT
> > sum(j) + sum(i) from mv_base_a;
> > ERROR:  expression containing an aggregate in it is not supported on
> > incrementally maintainable materialized view
> > ```
> > I can see some difficulties with division CREATE IMMV .... AS SELECT
> > 1/sum(i) from mv_base_a;  (sum(i) == 0 case), but adding &
> > multiplication should be ok, aren't they?
> >
> >
> > Overall, patchset looks mature, however it is far from being
> > committable due to lack of testing/feedback/discussion. There is only
> > one way to fix this... Test and discuss it!
> >
> >
> > [1] https://github.com/cloudberrydb/cloudberrydb
> 
> Hi! Small update: I tried to run a regression test and all
> IMMV-related tests failed on my vm. Maybe I'm doing something wrong, I
> will try to investigate.
> 
> Another suggestion: support for \d and \d+ commands in psql. With v34
> patchset applied, psql does not show anything IMMV-related in \d mode.
> 
> ```
> reshke=# \d m1
>            Materialized view "public.m1"
>  Column |  Type   | Collation | Nullable | Default
> --------+---------+-----------+----------+---------
>  i      | integer |           |          |
> Distributed by: (i)
> 
> 
> reshke=# \d+ m1
>                                      Materialized view "public.m1"
>  Column |  Type   | Collation | Nullable | Default | Storage |
> Compression | Stats target | Description
> --------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
>  i      | integer |           |          |         | plain   |
>     |              |
> View definition:
>  SELECT t1.i
>    FROM t1;
> Distributed by: (i)
> Access method: heap
> 
> ```
> 
> Output should be 'Incrementally materialized view "public.m1"' IMO.


-- 
Yugo NAGATA <nagata@sraoss.co.jp>