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/