Thread: Re: Remove unlogged materialized view persistence handling
On Fri, 11 Oct 2024 10:35:17 +0800 px shi <spxlyy123@gmail.com> wrote: > Hi, > > Since materialized views do not support the unlogged type, I think that we > should remove the logic and handling related to unlogged. Therefore, I have > submitted a patch. +1 Materialized view is introduced by 3bf3ab8c563 and at that UNLOGGED was allowed, and it is disallowed by another commit 3223b25ff73. However, it seems that the tab-complement is missed to fixed. The Assert for RELKIND_MATVIEW in heapam_hander.c was introduced in d25f519107b after that, but I think it can be also removed. Regards, Yugo Nagata -- Yugo Nagata <nagata@sraoss.co.jp>
+1
Materialized view is introduced by 3bf3ab8c563 and at that UNLOGGED was
allowed, and it is disallowed by another commit 3223b25ff73. However,
it seems that the tab-complement is missed to fixed.
The Assert for RELKIND_MATVIEW in heapam_hander.c was introduced in d25f519107b
after that, but I think it can be also removed.
Thank you for your feedback and support!
I also add this as a post in Commit Fest [0]
Regards,Pixian Shi
On Fri, 11 Oct 2024 14:44:43 +0800 px shi <spxlyy123@gmail.com> wrote: > > > > +1 > > > > Materialized view is introduced by 3bf3ab8c563 and at that UNLOGGED was > > allowed, and it is disallowed by another commit 3223b25ff73. However, > > it seems that the tab-complement is missed to fixed. Sorry, the comment above was not correct. The fix was about pasl's \d meta-command, not tab-complement. - if (tableinfo.relpersistence == 'u') - printfPQExpBuffer(&title, _("Unlogged materialized view \"%s.%s\""), - schemaname, relationname); - else - printfPQExpBuffer(&title, _("Materialized view \"%s.%s\""), - schemaname, relationname); + printfPQExpBuffer(&title, _("Materialized view \"%s.%s\""), + "Unlogged materialized view" is never printed since it cannot be created from the beginning of materialized view support, so this is a dead code to be removed. > > > > The Assert for RELKIND_MATVIEW in heapam_hander.c was introduced in > > d25f519107b > > after that, but I think it can be also removed. > > > > Thank you for your feedback and support! > > I also add this as a post in Commit Fest [0] > > > [0] https://commitfest.postgresql.org/50/5292/ I marked it as Ready-for-Committer. Regards, Yugo Nagata -- Yugo NAGATA <nagata@sraoss.co.jp>
On 2024/10/11 16:18, Yugo NAGATA wrote: > On Fri, 11 Oct 2024 14:44:43 +0800 > px shi <spxlyy123@gmail.com> wrote: > >>> >>> +1 >>> >>> Materialized view is introduced by 3bf3ab8c563 and at that UNLOGGED was >>> allowed, and it is disallowed by another commit 3223b25ff73. However, >>> it seems that the tab-complement is missed to fixed. > > Sorry, the comment above was not correct. > The fix was about pasl's \d meta-command, not tab-complement. > > - if (tableinfo.relpersistence == 'u') > - printfPQExpBuffer(&title, _("Unlogged materialized view \"%s.%s\""), > - schemaname, relationname); > - else > - printfPQExpBuffer(&title, _("Materialized view \"%s.%s\""), > - schemaname, relationname); > + printfPQExpBuffer(&title, _("Materialized view \"%s.%s\""), > + > > "Unlogged materialized view" is never printed since it cannot be created from the beginning > of materialized view support, so this is a dead code to be removed. Yes. I initially thought this code was kept intentionally because some older PostgreSQL versions supported unlogged materialized views, and even the latest psql might need to display them. However, this isn't the case. Unlogged materialized views were added and removed before the v9.3 release and were never supported in any official release. So there seems no need to keep this code. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
On 2024/10/11 17:06, Fujii Masao wrote: > > > On 2024/10/11 16:18, Yugo NAGATA wrote: >> On Fri, 11 Oct 2024 14:44:43 +0800 >> px shi <spxlyy123@gmail.com> wrote: >> >>>> >>>> +1 >>>> >>>> Materialized view is introduced by 3bf3ab8c563 and at that UNLOGGED was >>>> allowed, and it is disallowed by another commit 3223b25ff73. However, >>>> it seems that the tab-complement is missed to fixed. >> >> Sorry, the comment above was not correct. >> The fix was about pasl's \d meta-command, not tab-complement. >> >> - if (tableinfo.relpersistence == 'u') >> - printfPQExpBuffer(&title, _("Unlogged materialized view \"%s.%s\""), >> - schemaname, relationname); >> - else >> - printfPQExpBuffer(&title, _("Materialized view \"%s.%s\""), >> - schemaname, relationname); >> + printfPQExpBuffer(&title, _("Materialized view \"%s.%s\""), >> + >> >> "Unlogged materialized view" is never printed since it cannot be created from the beginning >> of materialized view support, so this is a dead code to be removed. > > Yes. I initially thought this code was kept intentionally because > some older PostgreSQL versions supported unlogged materialized views, > and even the latest psql might need to display them. However, > this isn't the case. Unlogged materialized views were added and > removed before the v9.3 release and were never supported in > any official release. So there seems no need to keep this code. I've pushed the patch. Thanks! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION