Re: [PATCH] Remove make_temptable_name_n()

On 2025-Oct-15, Aleksander Alekseev wrote:

> @@ -634,7 +611,17 @@ 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);
> + /*
> + * Create a name for the temporary diff table by appending an underscore
> + * followed by the given integer to the qualified temporary table name.
> + * 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.
> + */
> + diffname = psprintf("%s_%d", tempname, 2);

Hmm, but instead of keeping the comment about why this is bogus, why not
just fix it and remove the comment? You could do something like

nsp = get_namespace_name( .. );
diffname = psprintf("%s_%s_%d", nsp, RelationGetRelationName( .. ), 2);
tempname = quote_qualified_identifier(nsp, RelationGetRelationName( ... ));

and then that should be fairly okay, I think, keeping in mind that both
the names involved are internally-generated short strings -- something
like pg_temp_19.pg_temp_28356_2.

I think it would be better to rewrite this code not to rely on SPI.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/