Re: Implementing Incremental View Maintenance
| От | huyajun |
|---|---|
| Тема | Re: Implementing Incremental View Maintenance |
| Дата | |
| Msg-id | tencent_C101DDD099DC83DAF16B095820AE99BD6D09@qq.com обсуждение исходный текст |
| Ответ на | Re: Implementing Incremental View Maintenance (Yugo NAGATA <nagata@sraoss.co.jp>) |
| Ответы |
Re: Implementing Incremental View Maintenance
(Yugo NAGATA <nagata@sraoss.co.jp>)
|
| Список | pgsql-hackers |
2022年4月22日 下午1:58，Yugo NAGATA <nagata@sraoss.co.jp> 写道：On Fri, 22 Apr 2022 11:29:39 +0900
Yugo NAGATA <nagata@sraoss.co.jp> wrote:Hi,
On Fri, 1 Apr 2022 11:09:16 -0400
Greg Stark <stark@mit.edu> 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 <nagata@sraoss.co.jp>
<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
В списке pgsql-hackers по дате отправления: