Hi Nathan,
> > + {
> > + char *nsp =
> > get_namespace_name(RelationGetNamespace(tempRel));
> > + char *temprelname = RelationGetRelationName(tempRel);
> > + char *diffrelname = psprintf("%s_%d", temprelname, 2);
>
> I assume the intent of the extra set of curly brackets is to keep the
> declarations of these variables close to where they are used. In this
> case, the top of the function is only a few lines up, so IMHO we should
> declare them there and save a level of indentation.
>
> > + pfree(diffrelname);
> > + if (nsp)
> > + pfree(nsp);
>
> Any reason to be so careful about freeing these? We ordinarily let the
> memory context take care of freeing, and refresh_by_match_merge() looks no
> different.
These were just a matter of my personal preferences. I have no strong
opinion on this. Here is the updated patch.
--
Best regards,
Aleksander Alekseev
From f1f8da46e2bd160e4eec1774d6ef9fb93d8bc434 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 15 Oct 2025 13:28:24 +0300
Subject: [PATCH v5] Remove make_temptable_name_n()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The named function is used in matview.c only once and doesn't do much. Replace
it with a simple psprintf() call. On top of that, properly quote qualified
identifiers when generating diff table name.
Author: Aleksander Alekseev <[email protected]>
Reviewed-by: Nathan Bossart <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Reviewed-by: Shinya Kato <[email protected]>
Discussion: https://postgr.es/m/CAJ7c6TO3a5q2NKRsjdJ6sLf8isVe4aMaaX1-Hj2TdHdhFw8zRA%40mail.gmail.com
---
src/backend/commands/matview.c | 41 +++++++++++++---------------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 441de55ac24..7fb41307d9c 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -61,7 +61,6 @@ static void transientrel_shutdown(DestReceiver *self);
static void transientrel_destroy(DestReceiver *self);
static uint64 refresh_matview_datafill(DestReceiver *dest, Query *query,
const char *queryString, bool is_create);
-static char *make_temptable_name_n(char *tempname, int n);
static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
int save_sec_context);
static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence);
@@ -556,28 +555,6 @@ transientrel_destroy(DestReceiver *self)
pfree(self);
}
-
-/*
- * Given a qualified temporary table name, append an underscore followed by
- * the given integer, to make a new table name based on the old one.
- * The result is a palloc'd string.
- *
- * As coded, this would fail to make a valid SQL name if the given name were,
- * say, "FOO"."BAR". Currently, the table name portion of the input will
- * never be double-quoted because it's of the form "pg_temp_NNN", cf
- * make_new_heap(). But we might have to work harder someday.
- */
-static char *
-make_temptable_name_n(char *tempname, int n)
-{
- StringInfoData namebuf;
-
- initStringInfo(&namebuf);
- appendStringInfoString(&namebuf, tempname);
- appendStringInfo(&namebuf, "_%d", n);
- return namebuf.data;
-}
-
/*
* refresh_by_match_merge
*
@@ -620,6 +597,9 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
char *matviewname;
char *tempname;
char *diffname;
+ char *temprelname;
+ char *diffrelname;
+ char *nsp;
TupleDesc tupdesc;
bool foundUniqueIndex;
List *indexoidlist;
@@ -632,9 +612,18 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
matviewname = quote_qualified_identifier(get_namespace_name(RelationGetNamespace(matviewRel)),
RelationGetRelationName(matviewRel));
tempRel = table_open(tempOid, NoLock);
- tempname = quote_qualified_identifier(get_namespace_name(RelationGetNamespace(tempRel)),
- RelationGetRelationName(tempRel));
- diffname = make_temptable_name_n(tempname, 2);
+
+ /*
+ * Create names for the temporary table and diff table. For the diff
+ * table, append an underscore followed by the given integer to the
+ * relation name.
+ */
+ nsp = get_namespace_name(RelationGetNamespace(tempRel));
+ temprelname = RelationGetRelationName(tempRel);
+ diffrelname = psprintf("%s_%d", temprelname, 2);
+
+ tempname = quote_qualified_identifier(nsp, temprelname);
+ diffname = quote_qualified_identifier(nsp, diffrelname);
relnatts = RelationGetNumberOfAttributes(matviewRel);
--
2.43.0
