Обсуждение: [PATCH] Remove make_temptable_name_n()

Hi,

The proposed patch removes make_temptable_name_n() in matview.c. This
function is used only once and can be replaced with psprintf().

--
Best regards,
Aleksander Alekseev

On Fri, Oct 10, 2025 at 06:22:46PM +0300, Aleksander Alekseev wrote:
> The proposed patch removes make_temptable_name_n() in matview.c. This
> function is used only once and can be replaced with psprintf().

This dates back to commit cc1965a, and I see some discussion about this
function in the corresponding thread [0].  One thing I don't like about
the patch is that we lose the comment about relying on the name to never be
double-quoted.  IMHO that's worth retaining in some form.

[0] https://postgr.es/m/CAP7Qgm%3Djb3xkzQXfGtX9STx8fzd8EDDQ-oJ8ekcyeOud%2ByLCoA%40mail.gmail.com

-- 
nathan

Hi Nathan,

> This dates back to commit cc1965a, and I see some discussion about this
> function in the corresponding thread [0].  One thing I don't like about
> the patch is that we lose the comment about relying on the name to never be
> double-quoted.  IMHO that's worth retaining in some form.

Fair enough. Here is the corrected patch v2.

-- 
Best regards,
Aleksander Alekseev

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/