Remove unlogged materialized view persistence handling

Started by px shiover 1 year ago6 messages

spxlyy123@gmail.com

over 1 year ago

1 attachment(s)

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.

Regards,

Pixian Shi

Attachments:

0001-Remove-unlogged-materialized-view-persistence-handli.patchapplication/octet-stream; name=0001-Remove-unlogged-materialized-view-persistence-handli.patchDownload

From 9b403ac2c3fcf11d44711a07a54f10fb8d408814 Mon Sep 17 00:00:00 2001
From: shipixian <shipixian_yewu@cmss.chinamobile.com>
Date: Fri, 11 Oct 2024 10:18:29 +0800
Subject: [PATCH] Remove unlogged materialized view persistence handling

Signed-off-by: shipixian <shipixian_yewu@cmss.chinamobile.com>
---
 src/backend/access/heap/heapam_handler.c | 1 -
 src/bin/psql/describe.c                  | 8 ++------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 1c6da286d4..8c59b77b64 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -610,7 +610,6 @@ heapam_relation_set_new_filelocator(Relation rel,
 	if (persistence == RELPERSISTENCE_UNLOGGED)
 	{
 		Assert(rel->rd_rel->relkind == RELKIND_RELATION ||
-			   rel->rd_rel->relkind == RELKIND_MATVIEW ||
 			   rel->rd_rel->relkind == RELKIND_TOASTVALUE);
 		smgrcreate(srel, INIT_FORKNUM, false);
 		log_smgrcreate(newrlocator, INIT_FORKNUM);
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 6a36c91083..363a66e718 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1970,12 +1970,8 @@ describeOneTableDetails(const char *schemaname,
 							  schemaname, relationname);
 			break;
 		case RELKIND_MATVIEW:
-			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\""),
+							  schemaname, relationname);
 			break;
 		case RELKIND_INDEX:
 			if (tableinfo.relpersistence == 'u')
-- 
2.39.3 (Apple Git-145)

Yugo Nagata

nagata@sraoss.co.jp

over 1 year ago

In reply to: px shi (#1)

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.

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>

px shi

spxlyy123@gmail.com

over 1 year ago

In reply to: Yugo Nagata (#2)

Re: Remove unlogged materialized view persistence handling

+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]https://commitfest.postgresql.org/50/5292/

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

Regards,
Pixian Shi

Yugo NAGATA

nagata@sraoss.co.jp

over 1 year ago

In reply to: px shi (#3)

Re: Remove unlogged materialized view persistence handling

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>

Fujii Masao

masao.fujii@oss.nttdata.com

over 1 year ago

In reply to: Yugo NAGATA (#4)

Re: Remove unlogged materialized view persistence handling

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

Fujii Masao

masao.fujii@oss.nttdata.com

about 1 year ago

In reply to: Fujii Masao (#5)

Re: Remove unlogged materialized view persistence handling

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