Обсуждение: quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed

Hi Hackers,

== Background ==

While reviewing a patch, I noticed that quoteOneName() duplicates the logic of quote_identifier() for adding double quotes to identifiers.  The main difference is that quoteOneName() does *not* honor the GUC `quote_all_identifiers`.

The typical usage pattern of quoteOneName() looks like this:

```

    // Define a local buffer with size MAX_QUOTED_NAME_LEN

    // MAX_QUOTED_NAME_LEN = MAX_NAMELEN * 2 + 3 to ensure no overflow

    char attname[MAX_QUOTED_NAME_LEN];

    // Add quotes and copy into the stack buffer

    quoteOneName(attname, RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]));

    // Copy the quoted identifier into a StringInfo

    appendStringInfoString(&querybuf, attname);

```

This pattern is expensive because:

* it allocates a larger-than-necessary buffer on the stack

* it incurs two pallocs and two data copies

Looking further, the common pattern around quote_identifier() is similar:

```

    appendStringInfoString(&relations, quote_identifier(relname));

```

This also incurs two pallocs and two copies: quote_identifier() allocates a temporary buffer and copies the quoted identifier into it, and then appendStringInfoString() may allocate and copy again.

== Idea ==

I'd like to propose introducing `appendStringInfoIdentifier()`, which adds the quoting logic directly to StringInfo.  The goals are:

* eventually deprecate quoteOneName() — I'm not aware of a reason why it should ignore the GUC `quote_all_identifiers`

* simplify the usage patterns for quoting identifiers

* avoid unnecessary intermediate buffers, pallocs, and data copies

== About the attached patch ==

Attached v1 is not intended to be the final version — it is mainly to demonstrate the idea and get feedback on the design direction.

* 0001 implements `appendStringInfoIdentifier()` and uses it in a few places

* 0002 switches ri_triggers.c to use it, resolving a complicated usage pattern and showing a path toward removing quoteOneName()

Comments and suggestions on the overall direction would be very welcome.

Hi,

> -                appendStringInfo(&buffer, _("text search configuration %s"),
> -                                 quote_qualified_identifier(nspname,
> -                                                            NameStr(cfgForm->cfgname)));
> +                appendStringInfoQualifiedIdentifier(&buffer,
> +                                                    _("text search configuration "),
> +                                                    nspname, NameStr(cfgForm->cfgname), NULL);
>                  ReleaseSysCache(tup);
>                  break;
>              }

This doesn't work from a i18n point of view.  In the original
formulation, the translator is free to place the %s wherever it suits
the language.  In the new one there's no such freedom: the name will be
appended at the end.  Some existing translations:

ko.po:msgid "text search configuration %s"
ko.po-msgstr "%s 전문 검색 구성"

tr.po:msgid "text search configuration %s"
tr.po-msgstr "%s metin arama yapılandırması"