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/