public inbox for [email protected]
help / color / mirror / Atom feedFrom: huyajun <[email protected]>
To: Yugo NAGATA <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Greg Stark <[email protected]>
Cc: Zhihong Yu <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: [email protected] <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Andy Fan <[email protected]>
Cc: Tatsuo Ishii <[email protected]>
Cc: Thomas Munro <[email protected]>
Subject: Re: Implementing Incremental View Maintenance
Date: Wed, 29 Jun 2022 17:56:39 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CALNJ-vTaGk4AvjrCPPkW_GyWQ+KeW_dFA=hakcxfy=rGF28TWQ@mail.gmail.com>
<CALNJ-vRSVtqZDXU6-DjdycFkjonxKs2TbBFb=W=W9Y7KG0rRwA@mail.gmail.com>
<OS0PR01MB56829A65926C0FF7CF00F5EB82D29@OS0PR01MB5682.jpnprd01.prod.outlook.com>
<[email protected]>
<OS0PR01MB5682B499319EBEB59F416AD582619@OS0PR01MB5682.jpnprd01.prod.outlook.com>
<[email protected]>
<20220113102342.mlhknuy6ppypflq3@jrouhaud>
<[email protected]>
<CALNJ-vRkr8YbHMwWCo_PDGetzHZ68kxO+9gHpMmVjL=ws=iKYg@mail.gmail.com>
<[email protected]>
<CALNJ-vQyjRkCUbdKXHV-ReBpH_sEkfC2OE3cjx9wKjPm41a8OQ@mail.gmail.com>
<[email protected]>
<CAM-w4HM86ce16Sk9A9MRy4xsLk3a3WjNYZct78yEbaR3uajz4Q@mail.gmail.com>
<[email protected]>
<[email protected]>
> 2022年4月22日 下午1:58，Yugo NAGATA <[email protected]> 写道：
>
> On Fri, 22 Apr 2022 11:29:39 +0900
> Yugo NAGATA <[email protected]> wrote:
>
>> Hi,
>>
>> On Fri, 1 Apr 2022 11:09:16 -0400
>> Greg Stark <[email protected]> wrote:
>>
>>> This patch has bitrotted due to some other patch affecting trigger.c.
>>>
>>> Could you post a rebase?
>>>
>>> This is the last week of the CF before feature freeze so time is of the essence.
>>
>> I attached a rebased patch-set.
>>
>> Also, I made the folowing changes from the previous.
>>
>> 1. Fix to not use a new deptye
>>
>> In the previous patch, we introduced a new deptye 'm' into pg_depend.
>> This deptype was used for looking for IVM triggers to be removed at
>> REFRESH WITH NO DATA. However, we decided to not use it for reducing
>> unnecessary change in the core code. Currently, the trigger name and
>> dependent objclass are used at that time instead of it.
>>
>> As a result, the number of patches are reduced to nine from ten.
>
>
>> 2. Bump the version numbers in psql and pg_dump
>>
>> This feature's target is PG 16 now.
>
> Sorry, I revert this change. It was too early to bump up the
> version number.
>
> --
> Yugo NAGATA <[email protected]>
>
> <v27-0001-Add-a-syntax-to-create-Incrementally-Maintainabl.patch><v27-0002-Add-relisivm-column-to-pg_class-system-catalog.patch><v27-0003-Allow-to-prolong-life-span-of-transition-tables-.patch><v27-0004-Add-Incremental-View-Maintenance-support-to-pg_d.patch><v27-0005-Add-Incremental-View-Maintenance-support-to-psql.patch><v27-0006-Add-Incremental-View-Maintenance-support.patch><v27-0007-Add-aggregates-support-in-IVM.patch><v27-0008-Add-regression-tests-for-Incremental-View-Mainte.patch><v27-0009-Add-documentations-about-Incremental-View-Mainte.patch>
Hi, Nagata-san
I read your patch with v27 version and has some new comments,I want to discuss with you.
1. How about use DEPENDENCY_INTERNAL instead of DEPENDENCY_AUTO
when record dependence on trigger created by IMV.( related code is in the end of CreateIvmTrigger)
Otherwise, User can use sql to drop trigger and corrupt IVM, DEPENDENCY_INTERNAL is also semantically more correct
Crash case like:
create table t( a int);
create incremental materialized view s as select * from t;
drop trigger "IVM_trigger_XXXX”;
Insert into t values(1);
2. In get_matching_condition_string, Considering NULL values, we can not use simple = operator.
But how about 'record = record', record_eq treat NULL = NULL
it should fast than current implementation for only one comparation
Below is my simple implementation with this, Variables are named arbitrarily..
I test some cases it’s ok
static char *
get_matching_condition_string(List *keys)
{
StringInfoData match_cond;
ListCell *lc;
/* If there is no key columns, the condition is always true. */
if (keys == NIL)
return "true";
else
{
StringInfoData s1;
StringInfoData s2;
initStringInfo(&match_cond);
initStringInfo(&s1);
initStringInfo(&s2);
/* Considering NULL values, we can not use simple = operator. */
appendStringInfo(&s1, "ROW(");
appendStringInfo(&s2, "ROW(");
foreach (lc, keys)
{
Form_pg_attribute attr = (Form_pg_attribute) lfirst(lc);
char *resname = NameStr(attr->attname);
char *mv_resname = quote_qualified_identifier("mv", resname);
char *diff_resname = quote_qualified_identifier("diff", resname);
appendStringInfo(&s1, "%s", mv_resname);
appendStringInfo(&s2, "%s", diff_resname);
if (lnext(lc))
{
appendStringInfo(&s1, ", ");
appendStringInfo(&s2, ", ");
}
}
appendStringInfo(&s1, ")::record");
appendStringInfo(&s2, ")::record");
appendStringInfo(&match_cond, "%s operator(pg_catalog.=) %s", s1.data, s2.data);
return match_cond.data;
}
}
3. Consider truncate base tables, IVM will not refresh, maybe raise an error will be better
4. In IVM_immediate_before,I know Lock base table with ExclusiveLock is
for concurrent updates to the IVM correctly, But how about to Lock it when actually
need to maintain MV which in IVM_immediate_maintenance
In this way you don't have to lock multiple times.
5. Why we need CreateIndexOnIMMV, is it a optimize?
It seems like when maintenance MV,
the index may not be used because of our match conditions can’t use simple = operator
Looking forward to your early reply to answer my above doubts, thank you a lot!
Regards,
Yajun Hu
view thread (227+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Implementing Incremental View Maintenance
In-Reply-To: <[email protected]>
* 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