public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Postgres hackers <[email protected]>
Subject: quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed
Date: Mon, 17 Nov 2025 17:48:43 +0800
Message-ID: <CAEoWx2=g2RVkxXB=JzWphgfg4QGV+spaA3PQ1rBM2iMehrVvjg@mail.gmail.com> (raw)
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.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-Add-appendStringInfoIdentifier-and-use-it-in-a-fe.patch (10.5K, 3-v1-0001-Add-appendStringInfoIdentifier-and-use-it-in-a-fe.patch)
download | inline diff:
From c9e86872d98f4bc948fb370f582941d946887156 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Fri, 14 Nov 2025 21:44:29 +0800
Subject: [PATCH v1 1/2] Add appendStringInfoIdentifier and use it in a few
places
Author: Chao Li <[email protected]>
---
src/backend/catalog/objectaddress.c | 20 ++--
src/backend/commands/explain.c | 3 +-
src/backend/utils/adt/ri_triggers.c | 8 +-
src/backend/utils/adt/ruleutils.c | 142 +++++++++++++++++++++++-----
src/include/utils/builtins.h | 6 ++
5 files changed, 136 insertions(+), 43 deletions(-)
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index c75b7131ed7..fbb0b4f30f3 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -3611,9 +3611,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
else
nspname = get_namespace_name(cfgForm->cfgnamespace);
- 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;
}
@@ -4883,8 +4883,7 @@ getObjectIdentityParts(const ObjectAddress *object,
if (attr)
{
- appendStringInfo(&buffer, ".%s",
- quote_identifier(attr));
+ appendStringInfoIdentifier(&buffer, ".", attr, NULL);
if (objname)
*objname = lappend(*objname, attr);
}
@@ -5395,8 +5394,7 @@ getObjectIdentityParts(const ObjectAddress *object,
object->objectId);
break;
}
- appendStringInfoString(&buffer,
- quote_identifier(nspname));
+ appendStringInfoIdentifier(&buffer, NULL, nspname, NULL);
if (objname)
*objname = list_make1(nspname);
break;
@@ -5739,16 +5737,12 @@ getObjectIdentityParts(const ObjectAddress *object,
defacl = (Form_pg_default_acl) GETSTRUCT(tup);
username = GetUserNameFromId(defacl->defaclrole, false);
- appendStringInfo(&buffer,
- "for role %s",
- quote_identifier(username));
+ appendStringInfoIdentifier(&buffer, "for role ", username, NULL);
if (OidIsValid(defacl->defaclnamespace))
{
schema = get_namespace_name_or_temp(defacl->defaclnamespace);
- appendStringInfo(&buffer,
- " in schema %s",
- quote_identifier(schema));
+ appendStringInfoIdentifier(&buffer, " in schema ", schema, NULL);
}
else
schema = NULL;
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 7e699f8595e..cc979737845 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1705,8 +1705,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
explain_get_index_name(bitmapindexscan->indexid);
if (es->format == EXPLAIN_FORMAT_TEXT)
- appendStringInfo(es->str, " on %s",
- quote_identifier(indexname));
+ appendStringInfoIdentifier(es->str, " on ", indexname, NULL);
else
ExplainPropertyText("Index Name", indexname, es);
}
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 059fc5ebf60..9e13f526994 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -395,7 +395,6 @@ RI_FKey_check(TriggerData *trigdata)
{
quoteOneName(attname,
RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]));
-
appendStringInfo(&querybuf,
"SELECT 1 FROM (SELECT %s AS r FROM %s%s x",
attname, pk_only, pkrelname);
@@ -2095,7 +2094,6 @@ ri_GenerateQualCollation(StringInfo buf, Oid collation)
HeapTuple tp;
Form_pg_collation colltup;
char *collname;
- char onename[MAX_QUOTED_NAME_LEN];
/* Nothing to do if it's a noncollatable data type */
if (!OidIsValid(collation))
@@ -2111,10 +2109,8 @@ ri_GenerateQualCollation(StringInfo buf, Oid collation)
* We qualify the name always, for simplicity and to ensure the query is
* not search-path-dependent.
*/
- quoteOneName(onename, get_namespace_name(colltup->collnamespace));
- appendStringInfo(buf, " COLLATE %s", onename);
- quoteOneName(onename, collname);
- appendStringInfo(buf, ".%s", onename);
+ appendStringInfoIdentifier(buf, " COLLATE ", get_namespace_name(colltup->collnamespace), NULL);
+ appendStringInfoIdentifier(buf, ".", collname, NULL);
ReleaseSysCache(tp);
}
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 556ab057e5a..1d767deb6c4 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -13052,25 +13052,17 @@ printSubscripts(SubscriptingRef *sbsref, deparse_context *context)
}
}
-/*
- * quote_identifier - Quote an identifier only if needed
- *
- * When quotes are needed, we palloc the required space; slightly
- * space-wasteful but well worth it for notational simplicity.
- */
-const char *
-quote_identifier(const char *ident)
+static inline bool
+is_identifier_safe(const char *ident, int *nquotes)
{
/*
* Can avoid quoting if ident starts with a lowercase letter or underscore
* and contains only lowercase letters, digits, and underscores, *and* is
* not any SQL keyword. Otherwise, supply quotes.
*/
- int nquotes = 0;
bool safe;
- const char *ptr;
- char *result;
- char *optr;
+
+ *nquotes = 0;
/*
* would like to use <ctype.h> macros here, but they might yield unwanted
@@ -13078,7 +13070,7 @@ quote_identifier(const char *ident)
*/
safe = ((ident[0] >= 'a' && ident[0] <= 'z') || ident[0] == '_');
- for (ptr = ident; *ptr; ptr++)
+ for (const char *ptr = ident; *ptr; ptr++)
{
char ch = *ptr;
@@ -13092,7 +13084,7 @@ quote_identifier(const char *ident)
{
safe = false;
if (ch == '"')
- nquotes++;
+ (*nquotes)++;
}
}
@@ -13115,14 +13107,20 @@ quote_identifier(const char *ident)
safe = false;
}
- if (safe)
- return ident; /* no change needed */
+ return safe;
+}
- result = (char *) palloc(strlen(ident) + nquotes + 2 + 1);
+static inline const char *
+quote_identifier_internal(const char *ident, int nquotes, char **result)
+{
+ char *optr;
- optr = result;
+ if (*result == NULL)
+ *result = (char *) palloc(strlen(ident) + nquotes + 2 + 1);
+
+ optr = *result;
*optr++ = '"';
- for (ptr = ident; *ptr; ptr++)
+ for (const char *ptr = ident; *ptr; ptr++)
{
char ch = *ptr;
@@ -13133,7 +13131,107 @@ quote_identifier(const char *ident)
*optr++ = '"';
*optr = '\0';
- return result;
+ return *result;
+}
+
+/*
+ * quote_identifier - Quote an identifier only if needed
+ *
+ * When quotes are needed, we palloc the required space; slightly
+ * space-wasteful but well worth it for notational simplicity.
+ */
+const char *
+quote_identifier(const char *ident)
+{
+ int nquotes;
+ bool safe;
+ char *result = NULL;
+
+ safe = is_identifier_safe(ident, &nquotes);
+ if (safe)
+ return ident; /* no change needed */
+
+ return quote_identifier_internal(ident, nquotes, &result);
+}
+
+/*
+ * appendStringInfoIdentifier
+ * Append an SQL identifier to a StringInfo, quoting if required.
+ *
+ * This behaves like writing prefix + quote_identifier(ident) + suffix into
+ * the StringInfo, but emits the identifier directly into the buffer to avoid
+ * an intermediate palloc. prefix and suffix may be NULL.
+ *
+ * The identifier is copied verbatim if it is safe to leave unquoted; otherwise
+ * it is written with double quotes and embedded double quotes are doubled.
+ * Quoting rules are local to ruleutils, so this helper is defined here rather
+ * than in stringinfo.c.
+ */
+
+void
+appendStringInfoIdentifier(StringInfo str, const char *prefix,
+ const char *ident, const char *suffix)
+{
+ int nquotes;
+ bool safe;
+ int ident_len;
+ int prefix_len = 0;
+ int suffix_len = 0;
+
+ safe = is_identifier_safe(ident, &nquotes);
+ if (safe)
+ ident_len = strlen(ident);
+ else
+ ident_len = strlen(ident) + nquotes + 2; /* quotes + possible
+ * escapes */
+
+ if (prefix != NULL)
+ prefix_len = strlen(prefix);
+
+ if (suffix != NULL)
+ suffix_len = strlen(suffix);
+
+ enlargeStringInfo(str, ident_len + prefix_len + suffix_len + 1); /* +1 for trailing null */
+
+ if (prefix != NULL)
+ appendBinaryStringInfo(str, prefix, prefix_len);
+
+ if (safe)
+ appendBinaryStringInfo(str, ident, ident_len);
+ else
+ {
+ char *result = str->data + str->len;
+
+ (void) quote_identifier_internal(ident, nquotes, &result);
+ str->len += ident_len;
+ str->data[str->len] = '\0';
+ }
+
+ if (suffix != NULL)
+ appendBinaryStringInfo(str, suffix, suffix_len);
+}
+
+/*
+ * appendStringInfoQualifiedIdentifier
+ * Append a (possibly) qualified SQL identifier to a StringInfo.
+ *
+ * Writes prefix + qualifier + '.' + ident + suffix, quoting each identifier
+ * component if needed. If no qualifier is given, only ident (plus optional
+ * prefix/suffix) is appended. prefix and suffix may be NULL.
+ *
+ * This is a convenience wrapper around appendStringInfoIdentifier().
+ */
+void
+appendStringInfoQualifiedIdentifier(StringInfo str, const char *prefix,
+ const char *qualifier, const char *ident,
+ const char *suffix)
+{
+ if (qualifier)
+ {
+ appendStringInfoIdentifier(str, prefix, qualifier, ".");
+ prefix = NULL;
+ }
+ appendStringInfoIdentifier(str, prefix, ident, suffix);
}
/*
@@ -13150,8 +13248,8 @@ quote_qualified_identifier(const char *qualifier,
initStringInfo(&buf);
if (qualifier)
- appendStringInfo(&buf, "%s.", quote_identifier(qualifier));
- appendStringInfoString(&buf, quote_identifier(ident));
+ appendStringInfoIdentifier(&buf, NULL, qualifier, ".");
+ appendStringInfoIdentifier(&buf, NULL, ident, NULL);
return buf.data;
}
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index ce6285a2c03..ff4ba7265c2 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -84,6 +84,12 @@ extern void generate_operator_clause(StringInfo buf,
const char *leftop, Oid leftoptype,
Oid opoid,
const char *rightop, Oid rightoptype);
+extern void appendStringInfoIdentifier(StringInfo str, const char *prefix, const char *ident, const char *suffix);
+extern void appendStringInfoQualifiedIdentifier(StringInfo str,
+ const char *prefix,
+ const char *qualifier,
+ const char *ident,
+ const char *suffix);
/* varchar.c */
extern int bpchartruelen(char *s, int len);
--
2.39.5 (Apple Git-154)
[application/octet-stream] v1-0002-Apply-appendStringInfoIdentifier-in-ri_triggers.c.patch (21.3K, 4-v1-0002-Apply-appendStringInfoIdentifier-in-ri_triggers.c.patch)
download | inline diff:
From 05b67f72a8fc0b4578fed6fb7f82b8a423a7dd1d Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Mon, 17 Nov 2025 12:09:12 +0800
Subject: [PATCH v1 2/2] Apply appendStringInfoIdentifier in ri_triggers.c
Author: Chao Li <[email protected]>
---
src/backend/commands/matview.c | 4 +-
src/backend/utils/adt/ri_triggers.c | 188 +++++++++++++---------------
src/backend/utils/adt/ruleutils.c | 28 ++++-
src/include/utils/builtins.h | 9 +-
4 files changed, 119 insertions(+), 110 deletions(-)
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index ef7c0d624f1..f1ad5a8e059 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -797,9 +797,9 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
NameStr(attr->attname));
generate_operator_clause(&querybuf,
- leftop, attrtype,
+ NULL, leftop, attrtype, false,
op,
- rightop, attrtype);
+ NULL, rightop, attrtype, false);
foundUniqueIndex = true;
}
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 9e13f526994..3cc4dc2b7c8 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -196,9 +196,9 @@ static void quoteOneName(char *buffer, const char *name);
static void quoteRelationName(char *buffer, Relation rel);
static void ri_GenerateQual(StringInfo buf,
const char *sep,
- const char *leftop, Oid leftoptype,
+ const char *leftopprefix, const char *leftop, Oid leftoptype, bool quoteleftop,
Oid opoid,
- const char *rightop, Oid rightoptype);
+ const char *rightopprefix, const char *rightop, Oid rightoptype, bool quoterightop);
static void ri_GenerateQualCollation(StringInfo buf, Oid collation);
static int ri_NullCheck(TupleDesc tupDesc, TupleTableSlot *slot,
const RI_ConstraintInfo *riinfo, bool rel_is_pk);
@@ -358,7 +358,7 @@ RI_FKey_check(TriggerData *trigdata)
{
StringInfoData querybuf;
char pkrelname[MAX_QUOTED_REL_NAME_LEN];
- char attname[MAX_QUOTED_NAME_LEN];
+ const char *attname;
char paramname[16];
const char *querysep;
Oid queryoids[RI_MAX_NUMKEYS];
@@ -393,11 +393,11 @@ RI_FKey_check(TriggerData *trigdata)
quoteRelationName(pkrelname, pk_rel);
if (riinfo->hasperiod)
{
- quoteOneName(attname,
- RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]));
+ attname = RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]);
+ appendStringInfoIdentifier(&querybuf, "SELECT 1 FROM (SELECT ", attname, " AS r FROM ");
appendStringInfo(&querybuf,
- "SELECT 1 FROM (SELECT %s AS r FROM %s%s x",
- attname, pk_only, pkrelname);
+ "%s%s x",
+ pk_only, pkrelname);
}
else
{
@@ -410,13 +410,12 @@ RI_FKey_check(TriggerData *trigdata)
Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
- quoteOneName(attname,
- RIAttName(pk_rel, riinfo->pk_attnums[i]));
+ attname = RIAttName(pk_rel, riinfo->pk_attnums[i]);
sprintf(paramname, "$%d", i + 1);
ri_GenerateQual(&querybuf, querysep,
- attname, pk_type,
+ NULL, attname, pk_type, true,
riinfo->pf_eq_oprs[i],
- paramname, fk_type);
+ NULL, paramname, fk_type, false);
querysep = "AND";
queryoids[i] = fk_type;
}
@@ -428,9 +427,9 @@ RI_FKey_check(TriggerData *trigdata)
appendStringInfoString(&querybuf, ") x1 HAVING ");
sprintf(paramname, "$%d", riinfo->nkeys);
ri_GenerateQual(&querybuf, "",
- paramname, fk_type,
+ NULL, paramname, fk_type, false,
riinfo->agged_period_contained_by_oper,
- "pg_catalog.range_agg", ANYMULTIRANGEOID);
+ NULL, "pg_catalog.range_agg", ANYMULTIRANGEOID, false);
appendStringInfoString(&querybuf, "(x1.r)");
}
@@ -528,7 +527,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
{
StringInfoData querybuf;
char pkrelname[MAX_QUOTED_REL_NAME_LEN];
- char attname[MAX_QUOTED_NAME_LEN];
+ const char *attname;
char paramname[16];
const char *querysep;
const char *pk_only;
@@ -563,11 +562,11 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
quoteRelationName(pkrelname, pk_rel);
if (riinfo->hasperiod)
{
- quoteOneName(attname, RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]));
-
+ attname = RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]);
+ appendStringInfoIdentifier(&querybuf, "SELECT 1 FROM (SELECT ", attname, " AS r FROM ");
appendStringInfo(&querybuf,
- "SELECT 1 FROM (SELECT %s AS r FROM %s%s x",
- attname, pk_only, pkrelname);
+ "%s%s x",
+ pk_only, pkrelname);
}
else
{
@@ -579,13 +578,12 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
{
Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
- quoteOneName(attname,
- RIAttName(pk_rel, riinfo->pk_attnums[i]));
+ attname = RIAttName(pk_rel, riinfo->pk_attnums[i]);
sprintf(paramname, "$%d", i + 1);
ri_GenerateQual(&querybuf, querysep,
- attname, pk_type,
+ NULL, attname, pk_type, true,
riinfo->pp_eq_oprs[i],
- paramname, pk_type);
+ NULL, paramname, pk_type, false);
querysep = "AND";
queryoids[i] = pk_type;
}
@@ -597,9 +595,9 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
appendStringInfoString(&querybuf, ") x1 HAVING ");
sprintf(paramname, "$%d", riinfo->nkeys);
ri_GenerateQual(&querybuf, "",
- paramname, fk_type,
+ NULL, paramname, fk_type, false,
riinfo->agged_period_contained_by_oper,
- "pg_catalog.range_agg", ANYMULTIRANGEOID);
+ NULL, "pg_catalog.range_agg", ANYMULTIRANGEOID, false);
appendStringInfoString(&querybuf, "(x1.r)");
}
@@ -757,7 +755,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
StringInfoData querybuf;
char pkrelname[MAX_QUOTED_REL_NAME_LEN];
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
- char attname[MAX_QUOTED_NAME_LEN];
+ const char *attname;
char periodattname[MAX_QUOTED_NAME_LEN];
char paramname[16];
const char *querysep;
@@ -784,13 +782,12 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
- quoteOneName(attname,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
+ attname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
sprintf(paramname, "$%d", i + 1);
ri_GenerateQual(&querybuf, querysep,
- paramname, pk_type,
+ NULL, paramname, pk_type, false,
riinfo->pf_eq_oprs[i],
- attname, fk_type);
+ NULL, attname, fk_type, true);
querysep = "AND";
queryoids[i] = pk_type;
}
@@ -828,7 +825,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
char *pk_only = pk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ?
"" : "ONLY ";
- quoteOneName(attname, RIAttName(fk_rel, riinfo->fk_attnums[riinfo->nkeys - 1]));
+ attname = RIAttName(fk_rel, riinfo->fk_attnums[riinfo->nkeys - 1]);
sprintf(paramname, "$%d", riinfo->nkeys);
appendStringInfoString(&querybuf, " AND NOT coalesce(");
@@ -837,9 +834,9 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
initStringInfo(&intersectbuf);
appendStringInfoChar(&intersectbuf, '(');
ri_GenerateQual(&intersectbuf, "",
- attname, fk_period_type,
+ NULL, attname, fk_period_type, true,
riinfo->period_intersect_oper,
- paramname, pk_period_type);
+ NULL, paramname, pk_period_type, false);
appendStringInfoChar(&intersectbuf, ')');
/* Find the remaining history */
@@ -857,22 +854,21 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
{
Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
- quoteOneName(attname,
- RIAttName(pk_rel, riinfo->pk_attnums[i]));
+ attname = RIAttName(pk_rel, riinfo->pk_attnums[i]);
sprintf(paramname, "$%d", i + 1);
ri_GenerateQual(&replacementsbuf, querysep,
- paramname, pk_type,
+ NULL, paramname, pk_type, false,
riinfo->pp_eq_oprs[i],
- attname, pk_type);
+ NULL, attname, pk_type, true);
querysep = "AND";
queryoids[i] = pk_type;
}
appendStringInfoString(&replacementsbuf, " FOR KEY SHARE OF y) y2)");
ri_GenerateQual(&querybuf, "",
- intersectbuf.data, fk_period_type,
+ NULL, intersectbuf.data, fk_period_type, false,
riinfo->agged_period_contained_by_oper,
- replacementsbuf.data, ANYMULTIRANGEOID);
+ NULL, replacementsbuf.data, ANYMULTIRANGEOID, false);
/* end of coalesce: */
appendStringInfoString(&querybuf, ", false)");
}
@@ -944,7 +940,7 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
{
StringInfoData querybuf;
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
- char attname[MAX_QUOTED_NAME_LEN];
+ const char *attname;
char paramname[16];
const char *querysep;
Oid queryoids[RI_MAX_NUMKEYS];
@@ -969,13 +965,12 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
- quoteOneName(attname,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
+ attname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
sprintf(paramname, "$%d", i + 1);
ri_GenerateQual(&querybuf, querysep,
- paramname, pk_type,
+ NULL, paramname, pk_type, false,
riinfo->pf_eq_oprs[i],
- attname, fk_type);
+ NULL, attname, fk_type, true);
querysep = "AND";
queryoids[i] = pk_type;
}
@@ -1050,7 +1045,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
StringInfoData querybuf;
StringInfoData qualbuf;
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
- char attname[MAX_QUOTED_NAME_LEN];
+ const char *attname;
char paramname[16];
const char *querysep;
const char *qualsep;
@@ -1081,16 +1076,15 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
- quoteOneName(attname,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
- appendStringInfo(&querybuf,
- "%s %s = $%d",
- querysep, attname, i + 1);
+ attname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
+ appendStringInfoString(&querybuf, querysep);
+ appendStringInfoIdentifier(&querybuf, " ", attname, " = ");
+ appendStringInfo(&querybuf, "$%d", i + 1);
sprintf(paramname, "$%d", j + 1);
ri_GenerateQual(&qualbuf, qualsep,
- paramname, pk_type,
+ NULL, paramname, pk_type, false,
riinfo->pf_eq_oprs[i],
- attname, fk_type);
+ NULL, attname, fk_type, true);
querysep = ",";
qualsep = "AND";
queryoids[i] = pk_type;
@@ -1239,7 +1233,7 @@ ri_set(TriggerData *trigdata, bool is_set_null, int tgkind)
{
StringInfoData querybuf;
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
- char attname[MAX_QUOTED_NAME_LEN];
+ const char *attname;
char paramname[16];
const char *querysep;
const char *qualsep;
@@ -1297,11 +1291,10 @@ ri_set(TriggerData *trigdata, bool is_set_null, int tgkind)
querysep = "";
for (int i = 0; i < num_cols_to_set; i++)
{
- quoteOneName(attname, RIAttName(fk_rel, set_cols[i]));
- appendStringInfo(&querybuf,
- "%s %s = %s",
- querysep, attname,
- is_set_null ? "NULL" : "DEFAULT");
+ attname = RIAttName(fk_rel, set_cols[i]);
+ appendStringInfoString(&querybuf, querysep);
+ appendStringInfoIdentifier(&querybuf, " ", attname, " = ");
+ appendStringInfoString(&querybuf, is_set_null ? "NULL" : "DEFAULT");
querysep = ",";
}
@@ -1314,14 +1307,13 @@ ri_set(TriggerData *trigdata, bool is_set_null, int tgkind)
Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
- quoteOneName(attname,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
+ attname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
sprintf(paramname, "$%d", i + 1);
ri_GenerateQual(&querybuf, qualsep,
- paramname, pk_type,
+ NULL, paramname, pk_type, false,
riinfo->pf_eq_oprs[i],
- attname, fk_type);
+ NULL, attname, fk_type, true);
qualsep = "AND";
queryoids[i] = pk_type;
}
@@ -1519,8 +1511,8 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
StringInfoData querybuf;
char pkrelname[MAX_QUOTED_REL_NAME_LEN];
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
- char pkattname[MAX_QUOTED_NAME_LEN + 3];
- char fkattname[MAX_QUOTED_NAME_LEN + 3];
+ const char *pkattname;
+ const char *fkattname;
RangeTblEntry *rte;
RTEPermissionInfo *pk_perminfo;
RTEPermissionInfo *fk_perminfo;
@@ -1615,9 +1607,9 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
sep = "";
for (int i = 0; i < riinfo->nkeys; i++)
{
- quoteOneName(fkattname,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
- appendStringInfo(&querybuf, "%sfk.%s", sep, fkattname);
+ fkattname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
+ appendStringInfoString(&querybuf, sep);
+ appendStringInfoIdentifier(&querybuf, "fk.", fkattname, NULL);
sep = ", ";
}
@@ -1631,8 +1623,6 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
" FROM %s%s fk LEFT OUTER JOIN %s%s pk ON",
fk_only, fkrelname, pk_only, pkrelname);
- strcpy(pkattname, "pk.");
- strcpy(fkattname, "fk.");
sep = "(";
for (int i = 0; i < riinfo->nkeys; i++)
{
@@ -1641,14 +1631,12 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
Oid pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
Oid fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
- quoteOneName(pkattname + 3,
- RIAttName(pk_rel, riinfo->pk_attnums[i]));
- quoteOneName(fkattname + 3,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
+ pkattname = RIAttName(pk_rel, riinfo->pk_attnums[i]);
+ fkattname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
ri_GenerateQual(&querybuf, sep,
- pkattname, pk_type,
+ "pk.", pkattname, pk_type, true,
riinfo->pf_eq_oprs[i],
- fkattname, fk_type);
+ "fk.", fkattname, fk_type, true);
if (pk_coll != fk_coll)
ri_GenerateQualCollation(&querybuf, pk_coll);
sep = "AND";
@@ -1658,16 +1646,15 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
* It's sufficient to test any one pk attribute for null to detect a join
* failure.
*/
- quoteOneName(pkattname, RIAttName(pk_rel, riinfo->pk_attnums[0]));
- appendStringInfo(&querybuf, ") WHERE pk.%s IS NULL AND (", pkattname);
+ pkattname = RIAttName(pk_rel, riinfo->pk_attnums[0]);
+ appendStringInfoIdentifier(&querybuf, ") WHERE pk.", pkattname, " IS NULL AND (");
sep = "";
for (int i = 0; i < riinfo->nkeys; i++)
{
- quoteOneName(fkattname, RIAttName(fk_rel, riinfo->fk_attnums[i]));
- appendStringInfo(&querybuf,
- "%sfk.%s IS NOT NULL",
- sep, fkattname);
+ fkattname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
+ appendStringInfoString(&querybuf, sep);
+ appendStringInfoIdentifier(&querybuf, "fk.", fkattname, " IS NOT NULL");
switch (riinfo->confmatchtype)
{
case FKCONSTR_MATCH_SIMPLE:
@@ -1814,8 +1801,8 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
char *constraintDef;
char pkrelname[MAX_QUOTED_REL_NAME_LEN];
char fkrelname[MAX_QUOTED_REL_NAME_LEN];
- char pkattname[MAX_QUOTED_NAME_LEN + 3];
- char fkattname[MAX_QUOTED_NAME_LEN + 3];
+ const char *pkattname;
+ const char *fkattname;
const char *sep;
const char *fk_only;
int save_nestlevel;
@@ -1852,9 +1839,9 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
sep = "";
for (i = 0; i < riinfo->nkeys; i++)
{
- quoteOneName(fkattname,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
- appendStringInfo(&querybuf, "%sfk.%s", sep, fkattname);
+ fkattname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
+ appendStringInfoString(&querybuf, sep);
+ appendStringInfoIdentifier(&querybuf, "fk.", fkattname, NULL);
sep = ", ";
}
@@ -1865,8 +1852,8 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
appendStringInfo(&querybuf,
" FROM %s%s fk JOIN %s pk ON",
fk_only, fkrelname, pkrelname);
- strcpy(pkattname, "pk.");
- strcpy(fkattname, "fk.");
+ /* strcpy(pkattname, "pk."); */
+ /* strcpy(fkattname, "fk."); */
sep = "(";
for (i = 0; i < riinfo->nkeys; i++)
{
@@ -1875,14 +1862,12 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
Oid pk_coll = RIAttCollation(pk_rel, riinfo->pk_attnums[i]);
Oid fk_coll = RIAttCollation(fk_rel, riinfo->fk_attnums[i]);
- quoteOneName(pkattname + 3,
- RIAttName(pk_rel, riinfo->pk_attnums[i]));
- quoteOneName(fkattname + 3,
- RIAttName(fk_rel, riinfo->fk_attnums[i]));
+ pkattname = RIAttName(pk_rel, riinfo->pk_attnums[i]);
+ fkattname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
ri_GenerateQual(&querybuf, sep,
- pkattname, pk_type,
+ "pk.", pkattname, pk_type, true,
riinfo->pf_eq_oprs[i],
- fkattname, fk_type);
+ "fk.", fkattname, fk_type, true);
if (pk_coll != fk_coll)
ri_GenerateQualCollation(&querybuf, pk_coll);
sep = "AND";
@@ -1903,10 +1888,9 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
sep = "";
for (i = 0; i < riinfo->nkeys; i++)
{
- quoteOneName(fkattname, RIAttName(fk_rel, riinfo->fk_attnums[i]));
- appendStringInfo(&querybuf,
- "%sfk.%s IS NOT NULL",
- sep, fkattname);
+ fkattname = RIAttName(fk_rel, riinfo->fk_attnums[i]);
+ appendStringInfoString(&querybuf, sep);
+ appendStringInfoIdentifier(&querybuf, "fk.", fkattname, " IS NOT NULL");
switch (riinfo->confmatchtype)
{
case FKCONSTR_MATCH_SIMPLE:
@@ -2064,13 +2048,15 @@ quoteRelationName(char *buffer, Relation rel)
static void
ri_GenerateQual(StringInfo buf,
const char *sep,
- const char *leftop, Oid leftoptype,
+ const char *leftopprefix, const char *leftop, Oid leftoptype, bool quoteleftop,
Oid opoid,
- const char *rightop, Oid rightoptype)
+ const char *rightopprefix, const char *rightop, Oid rightoptype, bool quoterightop)
{
appendStringInfo(buf, " %s ", sep);
- generate_operator_clause(buf, leftop, leftoptype, opoid,
- rightop, rightoptype);
+ generate_operator_clause(buf,
+ leftopprefix, leftop, leftoptype, quoteleftop,
+ opoid,
+ rightopprefix, rightop, rightoptype, quoterightop);
}
/*
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 1d767deb6c4..035e8441e61 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -13570,9 +13570,9 @@ generate_operator_name(Oid operid, Oid arg1, Oid arg2)
*/
void
generate_operator_clause(StringInfo buf,
- const char *leftop, Oid leftoptype,
+ const char *leftopprefix, const char *leftop, Oid leftoptype, bool quoteleftop,
Oid opoid,
- const char *rightop, Oid rightoptype)
+ const char *rightopprefix, const char *rightop, Oid rightoptype, bool quoterightop)
{
HeapTuple opertup;
Form_pg_operator operform;
@@ -13588,12 +13588,30 @@ generate_operator_clause(StringInfo buf,
nspname = get_namespace_name(operform->oprnamespace);
- appendStringInfoString(buf, leftop);
+ if (quoteleftop)
+ appendStringInfoIdentifier(buf, leftopprefix, leftop, NULL);
+ else
+ {
+ if (leftopprefix)
+ appendStringInfoString(buf, leftopprefix);
+ appendStringInfoString(buf, leftop);
+ }
+ /* appendStringInfoString(buf, leftop); */
if (leftoptype != operform->oprleft)
add_cast_to(buf, operform->oprleft);
- appendStringInfo(buf, " OPERATOR(%s.", quote_identifier(nspname));
+ /* appendStringInfo(buf, " OPERATOR(%s.", quote_identifier(nspname)); */
+ appendStringInfoIdentifier(buf, " OPERATOR(", nspname, ".");
appendStringInfoString(buf, oprname);
- appendStringInfo(buf, ") %s", rightop);
+ appendStringInfoString(buf, ") ");
+ if (quoterightop)
+ appendStringInfoIdentifier(buf, rightopprefix, rightop, NULL);
+ else
+ {
+ if (rightopprefix)
+ appendStringInfoString(buf, rightopprefix);
+ appendStringInfoString(buf, rightop);
+ }
+ /* appendStringInfo(buf, ") %s", rightop); */
if (rightoptype != operform->oprright)
add_cast_to(buf, operform->oprright);
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index ff4ba7265c2..fc4de4446ac 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -80,10 +80,15 @@ extern PGDLLIMPORT bool quote_all_identifiers;
extern const char *quote_identifier(const char *ident);
extern char *quote_qualified_identifier(const char *qualifier,
const char *ident);
+
+/* extern void generate_operator_clause(StringInfo buf, */
+/* const char *leftop, Oid leftoptype, */
+/* Oid opoid, */
+/* const char *rightop, Oid rightoptype); */
extern void generate_operator_clause(StringInfo buf,
- const char *leftop, Oid leftoptype,
+ const char *leftopprefix, const char *leftop, Oid leftoptype, bool quoteleftop,
Oid opoid,
- const char *rightop, Oid rightoptype);
+ const char *rightopprefix, const char *rightop, Oid rightoptype, bool quoterightop);
extern void appendStringInfoIdentifier(StringInfo str, const char *prefix, const char *ident, const char *suffix);
extern void appendStringInfoQualifiedIdentifier(StringInfo str,
const char *prefix,
--
2.39.5 (Apple Git-154)
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: quoteOneName() inconsistency with quote_all_identifiers — replacement API proposed
In-Reply-To: <CAEoWx2=g2RVkxXB=JzWphgfg4QGV+spaA3PQ1rBM2iMehrVvjg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox