From 58f859a9766c3ffe8ccc761a174020fa2a3f417a Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Tue, 1 Jun 2021 19:59:12 -0700 Subject: [PATCH v4] Avoid alias name collisions in REFRESH MATERIALIZED VIEW There are alias names such as "mv", "newdata", "newdata2", "diff" used in REFRESH MATERIALIZED VIEW .. CONCURRENTLY code. These names are so common that users can have them in the queries and refreshing materialized view can fail. This patch randomizes the alias names with MyProcPid. --- src/backend/commands/matview.c | 86 ++++++++++++++-------------------- 1 file changed, 36 insertions(+), 50 deletions(-) diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 172ec6e982..193c72f1b3 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -66,7 +66,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); -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); @@ -532,25 +531,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. - * - * This leaks memory through palloc(), which won't be cleaned up until the - * current memory context is freed. - */ -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 * @@ -592,13 +572,14 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, Relation tempRel; char *matviewname; char *tempname; - char *diffname; TupleDesc tupdesc; bool foundUniqueIndex; List *indexoidlist; ListCell *indexoidscan; int16 relnatts; Oid *opUsedForQual; + char mvalias[NAMEDATALEN]; + char newdataalias[NAMEDATALEN]; initStringInfo(&querybuf); matviewRel = table_open(matviewOid, NoLock); @@ -607,8 +588,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, tempRel = table_open(tempOid, NoLock); tempname = quote_qualified_identifier(get_namespace_name(RelationGetNamespace(tempRel)), RelationGetRelationName(tempRel)); - diffname = make_temptable_name_n(tempname, 2); - relnatts = RelationGetNumberOfAttributes(matviewRel); /* Open SPI context. */ @@ -629,13 +608,15 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "SELECT newdata FROM %s newdata " - "WHERE newdata IS NOT NULL AND EXISTS " - "(SELECT 1 FROM %s newdata2 WHERE newdata2 IS NOT NULL " - "AND newdata2 OPERATOR(pg_catalog.*=) newdata " - "AND newdata2.ctid OPERATOR(pg_catalog.<>) " - "newdata.ctid)", - tempname, tempname); + "SELECT _$newdata_%d FROM %s _$newdata_%d " + "WHERE _$newdata_%d IS NOT NULL AND EXISTS " + "(SELECT 1 FROM %s _$newdata2_%d WHERE _$newdata2_%d IS NOT NULL " + "AND _$newdata2_%d OPERATOR(pg_catalog.*=) _$newdata_%d " + "AND _$newdata2_%d.ctid OPERATOR(pg_catalog.<>) " + "_$newdata_%d.ctid)", + MyProcPid, tempname, MyProcPid, MyProcPid, + tempname, MyProcPid, MyProcPid, MyProcPid, + MyProcPid, MyProcPid, MyProcPid); if (SPI_execute(querybuf.data, false, 1) != SPI_OK_SELECT) elog(ERROR, "SPI_exec failed: %s", querybuf.data); if (SPI_processed > 0) @@ -661,10 +642,11 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, /* Start building the query for creating the diff table. */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "CREATE TEMP TABLE %s AS " - "SELECT mv.ctid AS tid, newdata " - "FROM %s mv FULL JOIN %s newdata ON (", - diffname, matviewname, tempname); + "CREATE TEMP TABLE _$difftbl_%d AS " + "SELECT _$mv_%d.ctid AS tid, _$newdata_%d " + "FROM %s _$mv_%d FULL JOIN %s _$newdata_%d ON (", + MyProcPid, MyProcPid, MyProcPid, matviewname, + MyProcPid, tempname, MyProcPid); /* * Get the list of index OIDs for the table from the relcache, and look up @@ -677,6 +659,8 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, foundUniqueIndex = false; indexoidlist = RelationGetIndexList(matviewRel); + snprintf(mvalias, sizeof(mvalias), "_$mv_%d", MyProcPid); + snprintf(newdataalias, sizeof(mvalias), "_$newdata_%d", MyProcPid); foreach(indexoidscan, indexoidlist) { @@ -756,9 +740,9 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, if (foundUniqueIndex) appendStringInfoString(&querybuf, " AND "); - leftop = quote_qualified_identifier("newdata", + leftop = quote_qualified_identifier(newdataalias, NameStr(attr->attname)); - rightop = quote_qualified_identifier("mv", + rightop = quote_qualified_identifier(mvalias, NameStr(attr->attname)); generate_operator_clause(&querybuf, @@ -785,10 +769,10 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, */ Assert(foundUniqueIndex); - appendStringInfoString(&querybuf, - " AND newdata OPERATOR(pg_catalog.*=) mv) " - "WHERE newdata IS NULL OR mv IS NULL " - "ORDER BY tid"); + appendStringInfo(&querybuf, + " AND _$newdata_%d OPERATOR(pg_catalog.*=) _$mv_%d) " + "WHERE _$newdata_%d IS NULL OR _$mv_%d IS NULL ORDER BY tid", + MyProcPid, MyProcPid, MyProcPid, MyProcPid); /* Create the temporary "diff" table. */ if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY) @@ -804,7 +788,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, /* Analyze the diff table. */ resetStringInfo(&querybuf); - appendStringInfo(&querybuf, "ANALYZE %s", diffname); + appendStringInfo(&querybuf, "ANALYZE _$difftbl_%d", MyProcPid); if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY) elog(ERROR, "SPI_exec failed: %s", querybuf.data); @@ -813,20 +797,22 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, /* Deletes must come before inserts; do them first. */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "DELETE FROM %s mv WHERE ctid OPERATOR(pg_catalog.=) ANY " - "(SELECT diff.tid FROM %s diff " - "WHERE diff.tid IS NOT NULL " - "AND diff.newdata IS NULL)", - matviewname, diffname); + "DELETE FROM %s _$mv_%d WHERE ctid OPERATOR(pg_catalog.=) ANY " + "(SELECT _$diffas_%d.tid FROM _$difftbl_%d _$diffas_%d " + "WHERE _$diffas_%d.tid IS NOT NULL " + "AND _$diffas_%d._$newdata_%d IS NULL)", + matviewname, MyProcPid, MyProcPid, MyProcPid, + MyProcPid, MyProcPid, MyProcPid, MyProcPid); if (SPI_exec(querybuf.data, 0) != SPI_OK_DELETE) elog(ERROR, "SPI_exec failed: %s", querybuf.data); /* Inserts go last. */ resetStringInfo(&querybuf); appendStringInfo(&querybuf, - "INSERT INTO %s SELECT (diff.newdata).* " - "FROM %s diff WHERE tid IS NULL", - matviewname, diffname); + "INSERT INTO %s SELECT (_$diffas_%d._$newdata_%d).* " + "FROM _$difftbl_%d _$diffas_%d WHERE tid IS NULL", + matviewname, MyProcPid, MyProcPid, MyProcPid, + MyProcPid); if (SPI_exec(querybuf.data, 0) != SPI_OK_INSERT) elog(ERROR, "SPI_exec failed: %s", querybuf.data); @@ -837,7 +823,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, /* Clean up temp tables. */ resetStringInfo(&querybuf); - appendStringInfo(&querybuf, "DROP TABLE %s, %s", diffname, tempname); + appendStringInfo(&querybuf, "DROP TABLE _$difftbl_%d, %s", MyProcPid, tempname); if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY) elog(ERROR, "SPI_exec failed: %s", querybuf.data); -- 2.25.1