Thread: Re: Remove unlogged materialized view persistence handling

Re: Remove unlogged materialized view persistence handling

From
Yugo Nagata
Date:
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>



Re: Remove unlogged materialized view persistence handling

From
px shi
Date:
+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]


[0] https://commitfest.postgresql.org/50/5292/

Regards,
Pixian Shi

Re: Remove unlogged materialized view persistence handling

From
Yugo NAGATA
Date:
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>



Re: Remove unlogged materialized view persistence handling

From
Fujii Masao
Date:

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




Re: Remove unlogged materialized view persistence handling

From
Fujii Masao
Date:

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